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

honor order attribute in "finalize_distibution_options" group of entrypoints #1994

Merged
merged 2 commits into from
Feb 15, 2020
Merged

Conversation

con-f-use
Copy link
Contributor

@con-f-use con-f-use commented Feb 14, 2020

Summary of changes

The order attribute (if specified) is now honored for the targets of entrypoints in the setuptools.finalize_distribution_options group. Since introduction of these entrypoints in #1055 / #1877 there was a bug that caused the default order of 0 to be used regardless of what the target specified.

Closes #1993

This is my first PR in setuptools. I will add a towncrier changelog file and maybe a test of entrypoint execution order as soon as I have the chance and people here agree that my change is acceptable.

Pull Request Checklist

  • Changes have tests
  • News fragment added in changelog.d. See documentation for details

Copy link
Member

@jaraco jaraco left a comment

Choose a reason for hiding this comment

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

Looking good. Let me know if you would like help with towncrier entry.

@jaraco
Copy link
Member

jaraco commented Feb 15, 2020

Does this fix need to be applied to setuptools 44 (Python 2 supported)? If so, can you rebase it against the maint/44.x branch and target the PR there?

@con-f-use
Copy link
Contributor Author

con-f-use commented Feb 15, 2020

I think it does. The same problem caused by identical code exists there. I'll get to it.

@con-f-use con-f-use changed the base branch from master to maint/44.x February 15, 2020 15:43
@con-f-use con-f-use changed the base branch from maint/44.x to master February 15, 2020 15:44
@con-f-use
Copy link
Contributor Author

con-f-use commented Feb 15, 2020

Okay, turns out I have no idea how to do that with github. I though I could just cherrypick the two commits onto my fork's maint/44.x branch, then change the from and into base: branches here on github. But github will only let me change the latter. Little help, please?

Edit:
I found the common parent for both the master and maint/44.x branches of my fork with git merge-base, reset (--hard) the master branch to that, rebased maint/44.x onto that, effectively making my master into maint/44.x. I then cherry-picked my two commits onto that and force-pushed everything. Finally, I changed the into base: branch here on github. I hope that does the trick, if not let me know.

That was a lot of git magic for something so simple. I wonder if there is a better way.... 🐍

@con-f-use con-f-use changed the base branch from master to maint/44.x February 15, 2020 16:02
@jaraco jaraco merged commit ad8bfa8 into pypa:maint/44.x Feb 15, 2020
@jaraco
Copy link
Member

jaraco commented Mar 21, 2020

I see I merged this into maint/44.x, but I think I failed to release it or merge that into master.

@jaraco
Copy link
Member

jaraco commented Mar 21, 2020

Fix for this was released in v44.1.0 and v46.1.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

setuptools ignores order attribute of entrypoints inf finalize_distribution_options group
3 participants