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

ci: Update requirements-fixed.txt format #12631

Merged
merged 2 commits into from
Nov 9, 2023

Conversation

DatGizmo
Copy link
Contributor

  • The compile script output format was changed, updating requirements-fixed.txt to the new format

This PR requires #12626 to be merged, and might need a rebase.

@DatGizmo DatGizmo added the CI-disable Disable CI for this PR label Oct 11, 2023
@github-actions github-actions bot added the changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. label Oct 11, 2023
@NordicBuilder
Copy link
Contributor

NordicBuilder commented Oct 17, 2023

The changes to the 'requirements-fixed.txt' have been added to this PR.
All further changes to any of the requirements file will automatically be applied as long an 👀 is present.

Note: This comment is automatically posted and updated by the Comment GitHub Action.

@CLAassistant
Copy link

CLAassistant commented Oct 27, 2023

CLA assistant check
All committers have signed the CLA.

@DatGizmo DatGizmo marked this pull request as ready for review October 27, 2023 13:36
@DatGizmo DatGizmo requested review from karhama and removed request for ihansse, grho and shanthanordic October 27, 2023 13:46
Copy link
Contributor

@karhama karhama left a comment

Choose a reason for hiding this comment

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

lgtm

@@ -2,361 +2,132 @@
# This file is autogenerated by pip-compile with Python 3.8
# by the following command:
#
# pip-compile --allow-unsafe --output-file=requirements-fixed.txt --strip-extras ./bootloader/mcuboot/scripts/requirements.txt ./nrf/scripts/requirements-ci.txt ./nrf/scripts/requirements-extra.txt ./nrf/scripts/requirements.txt ./zephyr/scripts/requirements.txt
# pip-compile --allow-unsafe --annotation-style=line --output-file=/home/runner/work/sdk-nrf-testing/sdk-nrf-testing/ncs/nrf/scripts/requirements-fixed.txt --strip-extras ./bootloader/mcuboot/scripts/requirements.txt ./nrf/scripts/requirements-ci.txt ./nrf/scripts/requirements-extra.txt ./nrf/scripts/requirements.txt ./zephyr/scripts/requirements.txt
Copy link
Contributor

Choose a reason for hiding this comment

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

can we avoid invoking pip-compile with the full path to the out file ?
Ref: --output-file=/home/runner/work/sdk-nrf-testing/sdk-nrf-testing/ncs/nrf/scripts/requirements-fixed.txt

Especially because it's unfortunate to simply have changes to a file simply because a github workflow uses a different path.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think the path will change in the future.
This change is only highlighted here as I used a locally generated file to trigger the pipeline.
Will check if it is possible to drop the full path and only use relative format

Copy link
Contributor

Choose a reason for hiding this comment

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

if it's ok with Torsten, I propose we merge this as is (since the path is generated in a file that isn't in this PR)

and then look at printing a relative path when we 'backport/upmerge' this to sdk-nrf-next.

@DatGizmo DatGizmo marked this pull request as draft November 2, 2023 14:36
@thst-nordic
Copy link
Contributor

this is still needed right? take out of draft?

* The compile script output format was changed, updating
  requirements-fixed.txt to the new format

Signed-off-by: Sebastian Wezel <sebastian.wezel@nordicsemi.no>
This is an automated commit from github workflow by NordicBuilder

Signed-off-by: Nordic Builder <pylon@nordicsemi.no>
@DatGizmo DatGizmo marked this pull request as ready for review November 9, 2023 12:37
@rlubos rlubos merged commit 43d6b5a into nrfconnect:main Nov 9, 2023
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. CI-disable Disable CI for this PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants