Conversation
This should be unnecessary after this update.
5a848e5 to
6375a50
Compare
|
Later Edit: Some quick updates so that the fix to #170 is not limited just to GitHub |
|
Tests seem be failing due to the name changes of fields in the Action Menu, but not entirely sure why. |
|
Hi, Thanks for your PR and for all the time you put into RepoM. I appreciate it. However, it's hard for me to review your PR as you are solving quite a number of issues. I prefer smaller PR trying to solve one issue at the time. Because you put in so much effort, I'm going to write my feedback in different files right now. |
coenm
left a comment
There was a problem hiding this comment.
You did a lot, and again, thanks for that.
I hope you don't take this the wrong way but most of the changes are not necessary and take a lot time to review.
I thinks it better to discuss solutions, fixes or style changes upfront. Also make separate PRs if you address multiple issues.
Again, thanks for all the effort. I really hope you take the comments not personal.
| if (repository.Remotes.Count == 1 || forceSingle) | ||
| { | ||
| yield return new UserInterfaceRepositoryAction(name, repository) | ||
| menuTitle = "🔗 " + try_get_remote_repo_long_name(repository.Remotes[0].Url); |
There was a problem hiding this comment.
I really like the icons in the menu using RepositoryActionsV2.yaml configuration but not in the c# code.
...re/ActionMenu/Model/ActionMenus/BrowseRepository/RepositoryActionBrowseRepositoryV1Mapper.cs
Outdated
Show resolved
Hide resolved
| name = "Browse remote"; | ||
| } | ||
| string menuTitle; | ||
| //var menuTitle = await context.RenderStringAsync(action.Name).ConfigureAwait(false); |
There was a problem hiding this comment.
It's no longer gettng that text that way. It's getting it in a different way, depending on whether there is one or more remotes. See lines 32-52.
| } | ||
| } | ||
|
|
||
| private static string try_get_remote_repo_long_name(string remoteUrl) |
There was a problem hiding this comment.
I like that you are trying to solve/generate a remote name but i'm not sure this is the method I want
...M.ActionMenu.Core/ActionMenu/Model/ActionMenus/Git/Checkout/RepositoryActionGitCheckoutV1.cs
Outdated
Show resolved
Hide resolved
src/RepoM.Plugin.Clipboard/RepositoryAction/CopyToClipboardRepositoryCommandExecutor.cs
Outdated
Show resolved
Hide resolved
Sure, no worries. I'll break it down in more PRs in the future. |
|



This PR does the following:
_queryDictionarywas, in fact, a List. No more :)List<T>.Contains()checks.