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.

Sign up for GitHub

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

Jump to bottom

Split req.txt into req-dev.txt for development dependencies (#44) #90

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Split req.txt into req-dev.txt for development dependencies (#44) #90

wants to merge 1 commit into from

Conversation

MAVRICK-1
Copy link

@MAVRICK-1 MAVRICK-1 commented Jul 7, 2024 •
edited
Loading

Fixes #44

@glimchb can you review my PR

@MAVRICK-1 MAVRICK-1 requested a review from a team as a code owner July 7, 2024 04:33
@MAVRICK-1 MAVRICK-1 force-pushed the feature/split-req-txt-to-req-dev-44 branch from 5fe6b65 to a8a24fe Compare July 7, 2024 04:41
@glimchb
Copy link
Member

glimchb commented Jul 7, 2024

@MAVRICK-1 is the split correct? All build/test packages now in dev req txt ?

@MAVRICK-1
Copy link
Author

@glimchb sry my mistake
This will be
requirements-dev.txt

ansible-lint>=6.22.2
antsibull-changelog>=0.25.0
antsibull-docs>=2.7.0

And this will be
requirements.txt

python-dateutil>=2.8.2

@MAVRICK-1 MAVRICK-1 force-pushed the feature/split-req-txt-to-req-dev-44 branch from 9b19ad3 to 6ce9f8c Compare July 7, 2024 19:42
@MAVRICK-1
Copy link
Author

@glimchb

  • python-dateutil: Typically a runtime dependency.
  • antsibull-changelog: a development dependency (for generating changelogs).
  • antsibull-docs: a development dependency (for generating documentation).
  • ansible-lint: Development dependency (for linting).

glimchb
glimchb requested changes Jul 8, 2024
View reviewed changes
Copy link
Member

@glimchb glimchb left a comment

Choose a reason for hiding this comment

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

should you also change https://github.com/opiproject/ansible-opi-dpu/blob/main/Dockerfile#L4 ?

and other places that use this req.txt file ?

@glimchb
Copy link
Member

glimchb commented Jul 8, 2024

try to find existing examples how this should work properly...
for example https://github.com/PaloAltoNetworks/ansible-playbooks/blob/master/dev-requirements.txt

@glimchb glimchb mentioned this pull request Jul 8, 2024
Open
@MAVRICK-1 MAVRICK-1 force-pushed the feature/split-req-txt-to-req-dev-44 branch from 3277c90 to a3032c9 Compare July 10, 2024 12:41
@MAVRICK-1
Copy link
Author

@glimchb now it is okay ?

glimchb
glimchb reviewed Jul 10, 2024
View reviewed changes
Dockerfile Outdated Show resolved Hide resolved
@glimchb
Copy link
Member

glimchb commented Jul 10, 2024

@glimchb now it is okay ?

much better

@MAVRICK-1 MAVRICK-1 force-pushed the feature/split-req-txt-to-req-dev-44 branch from 182e771 to 832f88c Compare July 10, 2024 18:47
@MAVRICK-1 MAVRICK-1 requested a review from glimchb July 10, 2024 18:49
glimchb
glimchb reviewed Jul 10, 2024
View reviewed changes
Dockerfile Outdated Show resolved Hide resolved
@MAVRICK-1 MAVRICK-1 force-pushed the feature/split-req-txt-to-req-dev-44 branch from 7b03d6e to 0c49e69 Compare July 10, 2024 18:52
@MAVRICK-1 MAVRICK-1 requested a review from glimchb July 10, 2024 18:53
@glimchb glimchb linked an issue Jul 10, 2024 that may be closed by this pull request
pip: split req.txt to req-dev #44
Open
github-advanced-security[bot]
github-advanced-security bot found potential problems Jul 10, 2024
View reviewed changes
Dockerfile Outdated
COPY ./requirements.txt /tmp/requirements.txt
RUN pip install --no-cache-dir --disable-pip-version-check --requirement /tmp/requirements.txt
COPY ./requirements.txt ./requirements-dev.txt /tmp/
RUN pip install --no-cache-dir --disable-pip-version-check --requirement /tmp/requirements*.txt

Check warning

Code scanning / Scorecard

Pinned-Dependencies

score is 0: pipCommand not pinned by hash Click Remediation section below to solve this issue
@glimchb
Copy link
Member

glimchb commented Jul 10, 2024

solves #44

btw, solves #44 is not recognized by GitHub

try Fixes #44

glimchb
glimchb requested changes Jul 10, 2024
View reviewed changes
Copy link
Member

@glimchb glimchb left a comment

Choose a reason for hiding this comment

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

ci failed, please check

@MAVRICK-1
Copy link
Author

@glimchb Docker CLI FAIL HAS BEEN RESOLVED

glimchb
glimchb reviewed Jul 11, 2024
View reviewed changes
Dockerfile Outdated Show resolved Hide resolved
@MAVRICK-1
Copy link
Author

@glimchb done

github-advanced-security[bot]
github-advanced-security bot found potential problems Jul 12, 2024
View reviewed changes
Dockerfile
COPY ./requirements.txt /tmp/requirements.txt
RUN pip install --no-cache-dir --disable-pip-version-check --requirement /tmp/requirements.txt
COPY ./requirements.txt ./requirements-dev.txt /tmp/
RUN pip install --no-cache-dir --disable-pip-version-check --requirement /tmp/requirements.txt --requirement /tmp/requirements-dev.txt

Check warning

Code scanning / Scorecard

Pinned-Dependencies

score is 0: pipCommand not pinned by hash Click Remediation section below to solve this issue
@MAVRICK-1 MAVRICK-1 requested a review from glimchb July 12, 2024 16:33
@glimchb
Copy link
Member

glimchb commented Jul 16, 2024

@glimchb done

looks good, please rebase. I fixed ci, so it will pass now

@MAVRICK-1
Copy link
Author

@glimchb rebase done

@glimchb
Copy link
Member

glimchb commented Jul 16, 2024

@glimchb rebase done

Looks like ci passes!!!

last thing:
Can you squash all commits to single ine so we have bice liner history, please ?

@omcal @MAVRICK-1
fix: Split req.txt into req-dev.txt for development dependencies
6815225 
- Split req.txt
- Bumped python version (opiproject#91)

Signed-off-by: Rishi Mondal <[email protected]>
Signed-off-by: Boris Glimcher <[email protected]>
@MAVRICK-1 MAVRICK-1 force-pushed the feature/split-req-txt-to-req-dev-44 branch from cc6930f to 6815225 Compare July 17, 2024 18:04
@MAVRICK-1
Copy link
Author

@glimchb done 👍

@MAVRICK-1
Copy link
Author

@glimchb any updates ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@glimchb glimchb Awaiting requested review from glimchb

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

pip: split req.txt to req-dev
3 participants
@MAVRICK-1 @glimchb @omcal