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
style: require long urls for pypi and pythonhosted urls #8031
Conversation
fd39bf7
to
6181ee0
Compare
I'm open to feedback on the rubocop problem messages. Currently they are:
and
|
I would take out |
Looks good Before Homebrew/homebrew-core#58181:
After:
I'd say something like It might be overachieving, but if you extract the PyPI project name from the original url, you could also suggest they go directly to |
6181ee0
to
5f3f7d0
Compare
I like this. It's clearer about which url to use which is good. Ideally, the style check can suggest the correct url but I'm not sure how to have it figure out what the correct url is.
I like this but I worry that users will be confused and put |
@dtrodrigues how about this:
Looks like this on my terminal: |
Looks good @Rylan12 You can use automate it via the PyPI API to provide the exact url by looking at |
Thanks for the tip. I'll take a look. I'm not sure exactly how to implement that in ruby so if you have any tips let me know 😄 |
I think it's good for now as-is, especially because getting the exact url would need to be gated behind an If you want to roll that into this PR as well, I can take a look tomorrow or somebody else may be able to help today. |
As pointed out in #8032 (comment), implementing an autocorrect feature would require an audit rather than a rubocop. Personally, I think what's already here is enough. All URLs currently being used are already in the long-form, so I don't see a huge benefit to having it autocorrect. Plus, it already directs the user to the correct page, so it feels like an unnecessary optimization (at least at this point). ^ That's just my opinion, though, so I'd be happy to work on or take a look at something like that if it's desired. |
Looks good, let's |
Thanks to everyone for the comments and suggestions! |
Thanks for your hard work on this @dtrodrigues @Rylan12 @SeekingMeaning! |
Nice work @Rylan12! |
brew style
with your changes locally?brew tests
with your changes locally?See #8029 for the discussion behind this change
Require urls like
https://files.pythonhosted.org/packages/a0/b1/a01b2c3d4e5f6a7b8c9d0e1f2a3b4c5d6e7f8a9b0c1d2e3f4a5b6c7d8e9f/foo-0.1.tar.gz
instead ofhttps://files.pythonhosted.org/packages/source/f/foo/foo-0.1.tar.gz
orhttps://pypi.python.org/packages/source/foo/foo-0.1.tar.gz
Homebrew/core PR: Homebrew/homebrew-core#58181
Tagging @SeekingMeaning, @dtrodrigues and @SMillerDev