Skip to content
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

anime-downloader 5.0.1 #63211

Closed
wants to merge 2 commits into from
Closed

anime-downloader 5.0.1 #63211

wants to merge 2 commits into from

Conversation

bonkboykz
Copy link
Contributor

  • Have you followed the guidelines for contributing?
  • Have you checked that there aren't other open pull requests for the same formula update/change?
  • Have you built your formula locally with brew install --build-from-source <formula>, where <formula> is the name of the formula you're submitting?
  • Is your test running fine brew test <formula>, where <formula> is the name of the formula you're submitting?
  • Does your build pass brew audit --strict <formula> (after doing brew install <formula>)?

@BrewTestBot BrewTestBot added nodejs Node or npm use is a significant feature of the PR or issue python Python use is a significant feature of the PR or issue labels Oct 21, 2020
@bonkboykz
Copy link
Contributor Author

@dtrodrigues is there anything wrong with the pr?

@dtrodrigues
Copy link
Member

This can be merged after #62560 completes

@fxcoudert
Copy link
Member

@dtrodrigues thanks for putting it on hold!

Copy link
Member

@fxcoudert fxcoudert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Please rebase on top of master (because of the python upgrade revision bump)
  • Check whether the resources can be updated

@bonkboykz
Copy link
Contributor Author

* Please rebase on top of master (because of the python upgrade revision bump)

* Check whether the resources can be updated

Done

@bonkboykz
Copy link
Contributor Author

One of the tests is failing
Screen Shot 2020-10-23 at 16 33 50
with
Screen Shot 2020-10-23 at 16 33 21

Should I remove revision @fxcoudert ?

@dtrodrigues
Copy link
Member

Yes please

@dtrodrigues
Copy link
Member

dtrodrigues commented Oct 23, 2020

For reference, if you had run brew bump-formula-pr anime-downloader --version=5.0.1(https://docs.brew.sh/Manpage#bump-formula-pr-options-formula), it would have done the version bump, removed the revision, and also checked for resources that needed to be updated and updated urllib3 from 1.25.10 to 1.25.11. It also creates a new branch, pushes it, and creates a PR.

I've pushed the updated urlilb3 version to your branch.

@bonkboykz
Copy link
Contributor Author

For reference, if you had run brew bump-formula-pr anime-downloader --version=5.0.1, it would have done the version bump, removed the revision, and also seen and updated urllib3 from 1.25.10 to 1.25.11. I've pushed that updated resource to your branch.

I did, but it failed due to some network issues, which I later fixed, and, as far as I know, there's no way to continue bumping the version once it failed

@dtrodrigues
Copy link
Member

It should be possible to re-try if it failed along the way due to network issues. I'm wondering if after the failed network issue, the primary url had been updated in which case bump-formula-pr might not work. Resetting to the original one should allow you to re-run the command.

@bonkboykz
Copy link
Contributor Author

It should be possible to re-try if it failed along the way due to network issues. I'm wondering if after the failed network issue, the primary url had been updated in which case bump-formula-pr might not work. Resetting to the original one should allow you to re-run the command.

I was looking at this comment: Homebrew/brew#3150 (comment)

@dtrodrigues
Copy link
Member

The decision of whether or not to restore state was a deliberate one - there are cases where a different error occurred and one might want to continue from that position vs starting over from scratch. You may need to manually revert/manually delete the branch that was auto-created though. As the issue submitter pointed out, deleting the branch should allow you to re-run the command.

Thanks for your patience with this in the middle of our Python 3.9 migration and for your first contribution to Homebrew @bonkboykz!

@BrewTestBot
Copy link
Member

🤖 A scheduled task has triggered a merge.

@bonkboykz
Copy link
Contributor Author

The decision of whether or not to restore state was a deliberate one - there are cases where a different error occurred and one might want to continue from that position vs starting over from scratch. You may need to manually revert/manually delete the branch that was auto-created though. As the issue submitter pointed out, deleting the branch should allow you to re-run the command.

Thanks for your patience with this in the middle of our Python 3.9 migration and for your first contribution to Homebrew @bonkboykz!

Got it, I kinda rushed to the first solution I could come up with 😅 I'll try that next time

Thanks for the support, guys!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
nodejs Node or npm use is a significant feature of the PR or issue python Python use is a significant feature of the PR or issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants