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

build: ensure console scripts are available on PATH during build #221

Merged
merged 3 commits into from
Feb 7, 2021
Merged

build: ensure console scripts are available on PATH during build #221

merged 3 commits into from
Feb 7, 2021

Conversation

gaborbernat
Copy link
Contributor

@gaborbernat gaborbernat commented Feb 3, 2021

Resolves #214.

@gaborbernat gaborbernat requested a review from FFY00 as a code owner February 3, 2021 08:50
@gaborbernat gaborbernat requested a review from layday February 3, 2021 08:50
@gaborbernat gaborbernat added the bug Something isn't working label Feb 3, 2021
setup.cfg Show resolved Hide resolved
Copy link
Member

@FFY00 FFY00 left a comment

Choose a reason for hiding this comment

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

Thank you! I just have some minor comments.

src/build/__init__.py Outdated Show resolved Hide resolved
tests/test_projectbuilder.py Outdated Show resolved Hide resolved
tests/test_projectbuilder.py Outdated Show resolved Hide resolved
src/build/__init__.py Outdated Show resolved Hide resolved
src/build/env.py Outdated Show resolved Hide resolved
src/build/env.py Outdated Show resolved Hide resolved
src/build/__init__.py Outdated Show resolved Hide resolved
Signed-off-by: Bernát Gábor <[email protected]>
Signed-off-by: Bernát Gábor <[email protected]>
@gaborbernat
Copy link
Contributor Author

@layday @FFY00 I think now I've addressed all raised concerns 👍🏻 and CI is passing. @FFY00 do you want to add the changelog now?

Copy link
Member

@layday layday left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for working on this.

@gaborbernat
Copy link
Contributor Author

@FFY00 can we get this out the door?

Copy link
Member

@FFY00 FFY00 left a comment

Choose a reason for hiding this comment

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

Thanks 😊

@FFY00 FFY00 changed the title Ensure console scripts are available on PATH during build build: ensure console scripts are available on PATH during build Feb 7, 2021
@FFY00 FFY00 merged commit a0802bb into pypa:master Feb 7, 2021
@gaborbernat gaborbernat deleted the 1825-bin branch February 7, 2021 15:38
@gaborbernat
Copy link
Contributor Author

Considering this is a critical bugfix can we do a release with what we have (0.1.1?), thanks!

@FFY00
Copy link
Member

FFY00 commented Feb 7, 2021

Just a FYI, since we now have a changelog you can add an entry to CHANGELOG.rst in the PR. The order is supposed to be by relevance. I did it manually now.

@FFY00
Copy link
Member

FFY00 commented Feb 7, 2021

Considering this is a critical bugfix can we do a release with what we have (0.1.1?), thanks!

We can have a new release sure, but because there were a few behavior changes I think it should be 0.2.0, let's save 0.1.X for bugfixes.

@gaborbernat
Copy link
Contributor Author

Sure, I generally reserve minor versions for feature additions, not bug fixes, but don't mind.

The order is supposed to be by relevance.

How do you determine relevance? This feels very subjective and open for debate. I'd suggest something more objective.

@FFY00
Copy link
Member

FFY00 commented Feb 7, 2021

Sure, I generally reserve minor versions for feature additions, not bug fixes, but don't mind.

I am generally inclined to also increment minor versions when there are behavior changes that might break stuff, which is the case with us now resolving dependencies recursively.

How do you determine relevance? This feels very subjective and open for debate. I'd suggest something more objective.

Yes, it's subjective. I am not very picky here. The goal is to try to make things easier for users, if that requires two more sentences exchanged between maintainers, I am fine with that, if this becomes more we should revisit.

@FFY00
Copy link
Member

FFY00 commented Feb 7, 2021

Also, there was a breaking change https://pypa-build.readthedocs.io/en/stable/changelog.html#breaking-changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Console scripts not available during build
4 participants