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

PPBGDI_SB-3114: Added pipenv support - #minor #48

Merged
merged 1 commit into from
Mar 11, 2024

Conversation

ltflb-bgdi
Copy link
Contributor

  • Added pipenv support
  • Fixed flask test query-string

@github-actions github-actions bot added the bug label Mar 8, 2024
@github-actions github-actions bot changed the title PPBGDI_SB-3114: Added pipenv support PPBGDI_SB-3114: Added pipenv support - #minor Mar 8, 2024
@ltflb-bgdi ltflb-bgdi force-pushed the fix-BGDIINF_SB-3114-pipenv branch 4 times, most recently from 274544b to 2c0063d Compare March 8, 2024 16:49
@ltflb-bgdi ltflb-bgdi requested a review from ltshb March 8, 2024 16:50
Makefile Outdated
Comment on lines 28 to 29
PYTHON := $(PIPENV_RUN) python3
PYTHON := $(PIPENV_RUN)
Copy link
Contributor

Choose a reason for hiding this comment

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

One of the line is too much

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks. Done

Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
Makefile Outdated
Comment on lines 130 to 133
rm -rf dist
rm -rf build
rm -rf *.egg-info
rm -f .coverage
Copy link
Contributor

Choose a reason for hiding this comment

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

Why removing those ? they are created when building a package if I remember well ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

Pipfile Outdated
verify_ssl = true
name = "pypi"

[packages]
Copy link
Contributor

Choose a reason for hiding this comment

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

They should be all be put in dev-packages this library has no direct dependencies.
I would only keep the main depencencies and let pip handle the dependencies of dependencise, so you should only have the followings:

  • pylint
  • isort
  • yapft
  • coverage
  • django
  • flask
  • nose2
  • packaging
  • requests
  • twine

I might have forgotten some, needs to be tested. I would also only fix the minor version so using ~= instead of == as best practice.

@ltflb-bgdi ltflb-bgdi force-pushed the fix-BGDIINF_SB-3114-pipenv branch 8 times, most recently from c113944 to 92c30a3 Compare March 11, 2024 10:24
@ltflb-bgdi ltflb-bgdi requested a review from ltshb March 11, 2024 10:29
Makefile Outdated
Comment on lines 70 to 73
# .PHONY: ci
# ci: $(REQUIREMENTS)
# # Create virtual env with all packages for development using the Pipfile.lock
# pipenv sync --dev
Copy link
Contributor

Choose a reason for hiding this comment

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

Clean up

Added pipenv support
@ltflb-bgdi ltflb-bgdi force-pushed the fix-BGDIINF_SB-3114-pipenv branch from 92c30a3 to a5722db Compare March 11, 2024 10:58
@ltflb-bgdi ltflb-bgdi merged commit aaf87ed into master Mar 11, 2024
3 checks passed
@ltflb-bgdi ltflb-bgdi deleted the fix-BGDIINF_SB-3114-pipenv branch March 11, 2024 11:04
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.

2 participants