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

chore: Adds chardet as prod dependency #76

Merged
merged 3 commits into from Nov 6, 2021
Merged

chore: Adds chardet as prod dependency #76

merged 3 commits into from Nov 6, 2021

Conversation

jshwi
Copy link
Contributor

@jshwi jshwi commented Nov 2, 2021

Version 4.0.0 has been tested, so no breaking changes from 3.x.x

fixes side-effect of requirementslib not specifying chardet as dep
sarugaku/requirementslib#296 is still open

User will not need to install chardet manually to use this package

Version 4.0.0 has been tested, so no breaking changes from 3.x.x

fixes side-effect of `requirementslib` not specifying `chardet` as dep
sarugaku/requirementslib#296 is still open

User will not need to install `chardet` manually to use this package
@jshwi
Copy link
Contributor Author

jshwi commented Nov 2, 2021

Note: Fixes #73
Only aims to fix 1 issue, and does not make changes to any other dependencies

@m0ar
Copy link

m0ar commented Nov 4, 2021

@Madoshakalaka Would it be possible to merge this PR, or alternatively update the version of requirementslib to 1.6.1 that was released today? https://pypi.org/project/requirementslib/

It's currently breaking our CI pipeline :(

@bryant-finney bryant-finney self-requested a review November 4, 2021 14:58
Pipfile.lock Outdated
"sha256:ecd2c3fe726758037234c93df7e98deb257fd15c24c9180dacf1ef829da5f921",
"sha256:ef565033fa5a958e62796867b1df10c40263ea9ded87164d67572834e57a174d"
],
"markers": "python_version >= '3.9'",
Copy link
Collaborator

Choose a reason for hiding this comment

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

@jshwi Oops, was this intentional? It breaks pytest-mypy when trying to run pytest with python3.8

/cc @m0ar

Copy link
Contributor Author

@jshwi jshwi Nov 5, 2021

Choose a reason for hiding this comment

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

@bryant-finney This must be a result of the interpreter I was using, not intentional! edit: I installed mypy manually, I can see what happened now

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hm, not the cause of mypy installation, I'll look into this now and see what went wrong.

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 have pushed another commit for you to review :)

Copy link

Choose a reason for hiding this comment

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

MIght be ye olde --keep-outdated flag to pipenv install that's missing, pipenv likes sneaky side effects :)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Nice, thanks for digging in!

I'm trying to verify the Travis CI jobs are actually running / passing, but I'm not seeing any integrated GitHub widgets/components like I was expecting. Are you able to see the test status / logs / coverage results / etc?

I'm wondering if the CI integration needs to be updated after the migration from travis-ci.org to travis-ci.com

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@bryant-finney No probs!

I have the jobs saved, but the last few runs I've just tested with tox - I'm out of credits for Travis - and the tests have been on my fork. Not sure how the plan looks for this repo in particular.

I'm considering migrating the travis yaml file over to GH actions to view builds on my fork.

If you would like the logs for what I did collect I can email them your way.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Gotcha, yeah I was also considering forking to GitLab. I have a repo somewhere that uses tox with Docker executors for its CI jobs, but right now I'm not sure how to run tests in a Windows environment using GitLab. Does GitHub actions support windows runners?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

From what I know at this moment, GH actions should be able to do what Travis can do, I'm curious to look into it, but so far I've only used it for ubuntu builds. The good thing about actions is its integrated right into the repository, and widgets aren't a problem like you mentioned earlier.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@jshwi

Nice; I looked into it a little the other day. From what I understand, GitHub uses Azure-based infrastructure; so I would be very surprised if GitHub actions was unable to accommodate.

While looking, I saw that GitLab recently started providing windows executors as well. Since I'm already comfortable with GitLab pipelines, I went ahead and forked this project over to bryant.finney / pipenv-setup; that said, to me it seems better to use GitHub actions long-term.

So why don't we work on this in parallel? If CI/CD w/GitHub goes as quickly as one might hope, we can just drop the GitLab fork; otherwise, we can use the GitLab fork to run CI until we finish the GitHub actions integration. Does that sound reasonable to you?

Adding `chardet` to Pipfile seems to have changed the markers for `mypy`
Compatibility for 3.7 broken; `six-stub` package is needed; can be added
`chardet` moved to `dev-packages`
Locked with `pipenv lock --keep-outdated`
Less diff in Pipfile.lock vs b442e71 and 72eb526
No other dep changes needed
Copy link
Collaborator

@bryant-finney bryant-finney left a comment

Choose a reason for hiding this comment

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

I verified that pytest-mypy fails with the same error on master:

~/projects/github/pipenv-setup master*                                                                                   19:48:04
❯ tox
GLOB sdist-make: /home/bryant/projects/github/pipenv-setup/setup.py
[ ... ]
============================================================ FAILURES ============================================================
__________________________________________________________ test session __________________________________________________________
mypy exited with status 1.
______________________________________________________ pipenv_setup/main.py ______________________________________________________
16: error: Library stubs not installed for "six" (or incompatible with Python 3.7)
16: note: Hint: "python3 -m pip install types-six"
16: note: (or run "mypy --install-types" to install all missing stub packages)
16: note: See https://mypy.readthedocs.io/en/stable/running_mypy.html#missing-imports
_________________________________________________ pipenv_setup/pipfile_parser.py _________________________________________________
6: error: Library stubs not installed for "six" (or incompatible with Python 3.7)

I'll merge this PR now and open a second to fix mypy

@bryant-finney bryant-finney merged commit 2a53b70 into Madoshakalaka:master Nov 6, 2021
@jshwi jshwi deleted the chore/deps/chardet branch November 7, 2021 09:45
bryant-finney added a commit to bryant-finney/pipenv-setup that referenced this pull request Nov 9, 2021
Signed-off-by: Bryant Finney <finneybp@gmail.com>
bryant-finney added a commit to bryant-finney/pipenv-setup that referenced this pull request Nov 9, 2021
- add `dev` dependency for `types-six`
- require `pyparsing!=3.0.5` due to pyparsing/pyparsing#329
- update `pytest-mypy>=0.8`

Signed-off-by: Bryant Finney <finneybp@gmail.com>
bryant-finney added a commit to bryant-finney/pipenv-setup that referenced this pull request Nov 9, 2021
currently only installs `pyenv` and Python versions

Signed-off-by: Bryant Finney <finneybp@gmail.com>
bryant-finney added a commit to bryant-finney/pipenv-setup that referenced this pull request Nov 9, 2021
Signed-off-by: Bryant Finney <finneybp@gmail.com>
bryant-finney added a commit to bryant-finney/pipenv-setup that referenced this pull request Nov 9, 2021
- add `dev` dependency for `types-six`
- require `pyparsing!=3.0.5` due to pyparsing/pyparsing#329
- update `pytest-mypy>=0.8`

Signed-off-by: Bryant Finney <finneybp@gmail.com>
bryant-finney added a commit to bryant-finney/pipenv-setup that referenced this pull request Nov 9, 2021
Signed-off-by: Bryant Finney <finneybp@gmail.com>
bryant-finney added a commit to bryant-finney/pipenv-setup that referenced this pull request Nov 9, 2021
- add `dev` dependency for `types-six`
- require `pyparsing!=3.0.5` due to pyparsing/pyparsing#329
- update `pytest-mypy>=0.8`

Signed-off-by: Bryant Finney <finneybp@gmail.com>
bryant-finney added a commit to bryant-finney/pipenv-setup that referenced this pull request Nov 9, 2021
@jshwi jshwi mentioned this pull request Nov 11, 2021
bryant-finney added a commit that referenced this pull request Nov 16, 2021
* Resolve GitLab [#76]: Update dependencies to resolve failing tests

* ci: Adds .github/workflows/ci.yml

* docs: Adds CI status badge to README.md

Co-authored-by: Bryant Finney <finneybp@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants