-
Notifications
You must be signed in to change notification settings - Fork 5
Make poetry version configurable, fix Docker build Warnings #37
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
Conversation
nicoloboschi
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you very much, LGTM ; I'd add a test for the regexp
| # serach for "@generated by Poetry <version>" in first line | ||
| with poetry_lock_path.open("r") as f: | ||
| first_line = f.readline() | ||
| match = re.search(r"@generated by Poetry (\d+\.\d+\.\d+)", first_line) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would you mind to add a unit test with a real lock file for this detection?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would you mind to add a unit test with a real lock file for this detection?
I suggest, we lock the pyproject.toml in the tests folder using poetry itself and pin the poetry version in the workflow?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I created a dummy project, but the lock file did not get pushed because of the gitignore rules.
However, it will be created in the CI workflow - should it also be included in the repo?
The tests should run now properly again, at least they did on my fork: https://github.com/the-working-rene/poetry-dockerize-plugin/actions/runs/11104991328/job/30850201594
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd be better to include it in the repo but it's not a blocker for me
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alright, included it now. Also removed a not any more used method I added earlier
|
I'll need to look at it, but it may take some days to get back to it, just left the office for weekend.
Also tests still seem to fail, I'll also look into that.
|
d9543ec to
b9b5df5
Compare
new config entry "poetry-version" if not set, the poetry.lock is parsed for the version. If that fails, 1.8.3 is used as fallback
makes casing consistent for the 'FROM ... AS' instructions - avoids FromAsCasing waring, see https://docs.docker.com/reference/build-checks/from-as-casing/ build CMD as "Exec form" - avoids JSONArgsRecommended warning, see https://docs.docker.com/reference/build-checks/json-args-recommended/ only if configured entrypoint contains multiple entries, to avoid breaking entrypoints configured in "Shell form"
pin to 1.8.3, which is the latest currently released version
b9b5df5 to
170632b
Compare
see #36
also fixed some docker build warnings which annoyed me