-
-
Notifications
You must be signed in to change notification settings - Fork 7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Move the mastodon/*_cli files to mastodon/cli/* #24139
Conversation
8657fb8
to
45e84a9
Compare
This pull request has merge conflicts that must be resolved before it can be merged. |
45e84a9
to
fb5b7a5
Compare
This pull request has resolved merge conflicts and is ready for review. |
Since it's moving, it almost seems like it could be renamed it to |
This pull request has merge conflicts that must be resolved before it can be merged. |
fb5b7a5
to
e2c3ac2
Compare
This pull request has resolved merge conflicts and is ready for review. |
This pull request has merge conflicts that must be resolved before it can be merged. |
e2c3ac2
to
90db8ae
Compare
This pull request has resolved merge conflicts and is ready for review. |
9c0fd6b
to
5c48298
Compare
This pull request has merge conflicts that must be resolved before it can be merged. |
This pull request has resolved merge conflicts and is ready for review. |
This pull request has merge conflicts that must be resolved before it can be merged. |
ff0b00e
to
fcdb2b6
Compare
This pull request has resolved merge conflicts and is ready for review. |
This pull request has merge conflicts that must be resolved before it can be merged. |
3a62501
to
5fc8b01
Compare
This pull request has resolved merge conflicts and is ready for review. |
8433172
to
039afc0
Compare
0afdf42
to
a7c5f16
Compare
This pull request has merge conflicts that must be resolved before it can be merged. |
a7c5f16
to
ac5ac25
Compare
This pull request has resolved merge conflicts and is ready for review. |
ac5ac25
to
0ffe0a6
Compare
@Gargron @ClearlyClaire I'd love to get a review on this one. If approved, I want to rebase the other refactor+spec PR I have open against this post-file-move and start to get some of the CLI coverage improvements in. By raw LOC, the CLI classes are the largest contributor to missing coverage now. If the move is not approved, I can back out some of the spec+refactor stuff from the other PR and open more PRs against the current file names. Finally, I tried to make this as minimal a move/rename as possible, but if you see an even simpler way to do it let me know and I'll modify as desired. |
This pull request has merge conflicts that must be resolved before it can be merged. |
0ffe0a6
to
7d38936
Compare
This pull request has resolved merge conflicts and is ready for review. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new module/class/file names are definitely cleaner, but I'm not sure how much renaming this is worth the noise.
I'm tentatively for this, but I'd like @Gargron's opinion on it.
I'm expecting that this one-time move/rename is the noisiest lowest value (but necessary) part and the benefits show up in future PRs adding common base class, refactoring the helper, etc.
Agreed, but FWIW, work here done in part after this comment: #23978 (comment) |
I opened this draft -- #24134 -- and almost immediately realized the diff is challenging to review because so much changed/moved.
Opening this as an alternative to capture the bear minimum of file movements and changes here -- and will rebase that one for further refactor/specs/etc.
I think the only things here that are not straight renames or path adjustments are:
CLI
is a module name for all the subcommands, it can't also be a class name for the script entry point.I think everything else is just pure file movement and changing require paths.