Skip to content

Comments

Provide hints when an invalid license id is input#1634

Merged
finswimmer merged 1 commit intopython-poetry:masterfrom
JJJollyjim:license-suggestions
Jun 21, 2020
Merged

Provide hints when an invalid license id is input#1634
finswimmer merged 1 commit intopython-poetry:masterfrom
JJJollyjim:license-suggestions

Conversation

@JJJollyjim
Copy link
Contributor

@JJJollyjim JJJollyjim commented Nov 26, 2019 •
edited
Loading

  • Added tests for changed code.
  • [n/a?] Updated documentation for changed code.

Currently, the poetry new/init license field can be a little opaque to new users -- here is the actual output from my first time using poetry a few hours ago:

This command will guide you through creating your pyproject.toml config.

Package name [projects]: attoate  
Version [0.1.0]: 
Description []: 
Author [Jamie McClymont <[email protected]>, n to skip]: 
License []: GPLv3+ 
 Invalid license id: GPLv3+ 
License []: help
 Invalid license id: help 
License []: ?
 Invalid license id: ? 
License []: gpl-3-or-later
 Invalid license id: gpl-3-or-later 
License []: ?
 Invalid license id: ? 

-------- Some googling later --------

License []: GPL-3.0-or-later

This PR improves the error message to show a link to the SPDX license list, and in common cases, a best-effort guess at which license ids you might be looking for. For example:

invalid license id: some-wacky-license
Poetry uses SPDX license identifiers: https://spdx.org/licenses/

Or, if a common license id is recognised:

Invalid license id: that one from the apache web server
Poetry uses SPDX license identifiers: https://spdx.org/licenses/
Did you mean one of the following?
 * Apache-1.0
 * Apache-1.1
 * Apache-2.0

I'm not a python dev (as evidenced by the fact I didn't know what a license id was :P), feedback is welcome :)

finswimmer
finswimmer reviewed Dec 19, 2019
View reviewed changes
Copy link
Member

@finswimmer finswimmer left a comment

Choose a reason for hiding this comment

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

Hello @JJJollyjim ,

thanks a lot for this improvement of the user experience. I really like it!

I think it is ok if we integrate it into the master branch. Could your rebase your commit on that branch?

fin swimmer

@JJJollyjim JJJollyjim changed the base branch from develop to master December 19, 2019 08:06
@JJJollyjim
Copy link
Contributor Author

JJJollyjim commented Dec 19, 2019

Done, I believe :)

finswimmer
finswimmer previously approved these changes Dec 19, 2019
View reviewed changes
Copy link
Member

@finswimmer finswimmer left a comment

Choose a reason for hiding this comment

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

Perfect!

@finswimmer finswimmer added the area/error-handling Bad error messages/insufficient error handling label Dec 23, 2019
@finswimmer finswimmer requested a review from sdispater January 5, 2020 18:53
sdispater
sdispater previously requested changes Jan 6, 2020
View reviewed changes
@JJJollyjim JJJollyjim force-pushed the license-suggestions branch from 3a0e158 to 7276451 Compare January 9, 2020 13:14
@JJJollyjim JJJollyjim requested a review from sdispater January 9, 2020 13:16
@JJJollyjim
Copy link
Contributor Author

JJJollyjim commented Feb 8, 2020

Anything else I can do on this?

@brechtm
Copy link

brechtm commented Jun 18, 2020

This message is just a friendly reminder, since this PR seems ready to be included.

@finswimmer
Copy link
Member

finswimmer commented Jun 20, 2020

@JJJollyjim may I ask you to add a minor change and push again, so that the checks will run again? Afterwards I will be able to merge it.

@finswimmer finswimmer self-requested a review June 20, 2020 16:01
@JJJollyjim JJJollyjim force-pushed the license-suggestions branch from 7276451 to d80be26 Compare June 20, 2020 23:47
@JJJollyjim JJJollyjim force-pushed the license-suggestions branch from d80be26 to 65e042c Compare June 20, 2020 23:50
finswimmer
finswimmer approved these changes Jun 21, 2020
View reviewed changes
Copy link
Member

@finswimmer finswimmer left a comment

Choose a reason for hiding this comment

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

Great. Thanks a lot 👍

@finswimmer finswimmer removed the request for review from sdispater June 21, 2020 06:42
@finswimmer finswimmer dismissed sdispater’s stale review June 21, 2020 11:36

change is included

@finswimmer finswimmer merged commit cd6da2c into python-poetry:master Jun 21, 2020
@sdispater sdispater mentioned this pull request Jul 21, 2020
Merged
@github-actions
Copy link

github-actions bot commented Mar 1, 2024

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 1, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@finswimmer finswimmer finswimmer approved these changes

@sdispater sdispater sdispater left review comments

Assignees

No one assigned

Labels

area/error-handling Bad error messages/insufficient error handling

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@JJJollyjim @brechtm @finswimmer @sdispater