This is an archive of the discontinued LLVM Phabricator instance.

Make the version check PEP440-compliant
ClosedPublic

Authored by jroelofs on Apr 6 2022, 8:36 AM.

Details

Diff Detail

Repository
rLNT LNT

Event Timeline

jroelofs created this revision.Apr 6 2022, 8:36 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 6 2022, 8:36 AM
jroelofs requested review of this revision.Apr 6 2022, 8:36 AM

Is the change about handling cases where the project name is something that would not be valid for parse_version? Could you detail the commit message a little bit to explain what is currently not compliant?

jroelofs added a comment.EditedApr 6 2022, 11:33 AM

This showed up in one of our buildbots:

++ lnt check-no-errors lnt-submission.json
lib/python3.8/site-packages/pkg_resources/__init__.py:122: PkgResourcesDeprecationWarning: LNT 0.4.2.dev0 is an invalid version and will not be supported in a future release
  warnings.warn(
++ echo @@@@@@@
@@@@@@@

This showed up in one of our buildbots:

++ lnt check-no-errors lnt-submission.json
lib/python3.8/site-packages/pkg_resources/__init__.py:122: PkgResourcesDeprecationWarning: LNT 0.4.2.dev0 is an invalid version and will not be supported in a future release
  warnings.warn(
++ echo @@@@@@@
@@@@@@@

Ah right so I suggest a description along the liens of:

Call parse_version on the LNT version only to be PEP 440 compliant. Better wording welcome. LGTM with a better description

cmatthews accepted this revision.Apr 6 2022, 12:38 PM

Looks good, with the suggested commit message change.

This revision is now accepted and ready to land.Apr 6 2022, 12:38 PM

Seems I don't have permissions on the repo to commit this.

thopre added a comment.Apr 6 2022, 1:29 PM

Seems I don't have permissions on the repo to commit this.

I can commit for you. Are you happy with the commit description I proposed?

sounds good to me, thank you!

thopre added a comment.EditedApr 6 2022, 1:42 PM
This comment has been deleted.
lnt/lnttool/main.py
474–476

I've fixed some C++ism when applying the patch or many tests were failing.

This revision was automatically updated to reflect the committed changes.
jroelofs added inline comments.Apr 6 2022, 2:15 PM
lnt/lnttool/main.py
474–476

Good catch, thanks!