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

Fix version regex for multi-line version files #36

Merged
merged 1 commit into from Jul 31, 2022
Merged

Fix version regex for multi-line version files #36

merged 1 commit into from Jul 31, 2022

Conversation

mjpieters
Copy link
Contributor

@mjpieters mjpieters commented Apr 2, 2022

What do these changes do?

The version regex has several issues, primarily because it assumes the version file consists of a single line. This broke the Yarl 1.8 release; this PR fixes the issue.

  • Expand version tests to use a multi-line mock-module text as base.
  • Add a test with matching quote following version string to verify the backref is working
    correctly.
  • add re.MULTILINE flag so ^ matches right after a newline, not just the
    start of the text.
  • Use a negative lookahead assertion against the backref, instead of a backref
    inside a character class. Character classes don't support backrefs.
  • Use .search() instead of .match(); we need to find the pattern anywhere
    in the version file, not just the start.
  • Add missing INPUT_CHECK_REF env var to runs: section.

Checklist

  • I think the code is well written
  • Unit tests for the changes exist
  • Documentation reflects the changes

@codecov
Copy link

codecov bot commented Apr 2, 2022

Codecov Report

❗ No coverage uploaded for pull request base (master@72c6c5c). Click here to learn what that means.
The diff coverage is 100.00%.

❗ Current head d4c6e9e differs from pull request most recent head a503ba0. Consider uploading reports for the commit a503ba0 to get more accurate results

@@            Coverage Diff            @@
##             master      #36   +/-   ##
=========================================
  Coverage          ?   75.00%           
=========================================
  Files             ?        1           
  Lines             ?      136           
  Branches          ?       30           
=========================================
  Hits              ?      102           
  Misses            ?       28           
  Partials          ?        6           
Impacted Files Coverage Δ
get_releasenote.py 75.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us.

@mjpieters mjpieters mentioned this pull request Apr 2, 2022
4 tasks
@mjpieters
Copy link
Contributor Author

Why is there a Python version test matrix when the action only ever runs on 3.10?

@mjpieters
Copy link
Contributor Author

The pre-commit-based lint step also looks redundant (apart from the fact that it fails because it uses a git:// URL).

@mjpieters
Copy link
Contributor Author

I rebased this onto #37 so the CI steps at least pass.

@mjpieters
Copy link
Contributor Author

@asvetlov Pokedipoke. Any chance this can get reviewed and merged?

- Expand version tests to use a multi-line mock-module text as base.
- Add a test with matching quote following version string to verify the backref is working
  correctly.
- add `re.MULTILINE` flag so `^` matches right after a newline, not just the
  start of the text.
- Use a negative lookahead assertion against the backref, instead of a backref
  inside a character class. Character classes don't support backrefs.
- Use `.search()` instead of `.match()`; we need to find the pattern anywhere
  in the version file, not just the start.
@webknjaz webknjaz merged commit b0fcc7f into aio-libs:master Jul 31, 2022
@mjpieters mjpieters deleted the fix_version_regex branch August 8, 2022 13:43
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.

None yet

2 participants