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

gherkin (Python): Updating to Python 3 (standard minimum supported version is now 3.7) #1982

Closed
wants to merge 13 commits into from

Conversation

jsa34
Copy link

@jsa34 jsa34 commented May 17, 2022

…but technically the code supports Python 3.3+)

Summary

A general code clean up to remove references to Python2 and any other unsupported versions. Technically the code supports Python 3.3+, but unintentionally.

Details

Motivation and Context

Prepare for feature changes without having to support unnecessary backwards-compatability

How Has This Been Tested?

Unit tests run as expected

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue).
  • New feature (non-breaking change which adds functionality).
  • Breaking change (fix or feature that would cause existing functionality to not work as expected).

Checklist:

  • The change has been ported to Java.
  • The change has been ported to Ruby.
  • The change has been ported to JavaScript.
  • The change has been ported to Go.
  • The change has been ported to .NET.
  • I've added tests for my code.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have updated the CHANGELOG accordingly.

…but technically the code supports Python 3.3+)
@jsa34
Copy link
Author

jsa34 commented May 17, 2022

Apologies - I accidentally made a branch against the cucumber project itself, rather than a fork! (Slap on the wrist and will do so going forward!)

@jsa34 jsa34 marked this pull request as ready for review May 18, 2022 08:18
@jsa34
Copy link
Author

jsa34 commented May 18, 2022

Build fails after merging "main" into this branch - not sure about this cause:

Working copy is dirty. Please run 'source scripts/functions.sh && rsync_files' and commit modified files.
Found: 
 M gherkin/python/default.mk
make: *** [Makefile:34: check_synced] Error 1

@aurelien-reeves
Copy link
Contributor

What is happening here is that, as part of the monorepo, a few files are shared across all components.

default.mk is one of them.

In order to update such file, you have to actually update the one in .templates\python (or the language you are working on), then synchronize all the files in the repo using source scripts/functions.sh && rsync_files in bash. That will update (synchronize) all the files that are shared across the repo. Then you can commit and push the changes.

If you are not sure, we could pair if you want. If so, do not hesitate to contact me on slack :)

@jsa34
Copy link
Author

jsa34 commented May 18, 2022

Thank you so much for your assistance, @aurelien-reeves ! The build seems happy again :)

@aurelien-reeves
Copy link
Contributor

So, the big question here is: do everyone agree to end support for python 2?

@aslakhellesoy @jenisys?

Myself, I would agree. But I am not really a python developer so I may miss some background here

@jsa34
Copy link
Author

jsa34 commented May 19, 2022 via email

Generate parser.py from razor file, rather than manual.
A few missed tidy ups (e.g. defining an attribute in __init__ instead of dynamically when calling a method; 'rU' for read is deprecated and U is default for Python 3)
@jsa34
Copy link
Author

jsa34 commented May 19, 2022

Resolved the issue with using the Razor file to generate parser.py.

@jsa34
Copy link
Author

jsa34 commented May 20, 2022

Requirement to continue support for Python 2.

@jsa34 jsa34 closed this May 20, 2022
@luke-hill luke-hill deleted the gherkin-python-three-update branch August 15, 2023 11:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants