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

Quick fix for #89: Bump pyre-extensions version in requirements.txt #90

Closed

Conversation

abishekvashok
Copy link
Contributor

@abishekvashok abishekvashok commented Mar 3, 2023

Bump pyre-extensions version in requirements.txt
Since, these are extensions are for the typing module, I think we can remove the version information as well.

Pre-submission checklist

  • I've ran the following linters locally and fixed lint errors related to the files I modified in this PR
    • black .
    • usort format .
    • flake8
  • I've installed dev dependencies pip install -r requirements-dev.txt and completed the following:
    • I've ran tests with ./scripts/run-tests.sh and made sure all tests are passing

Summary

A simple version bump for pyre-extensions.
In order to prevent dependency issues in sapp, we decided to pin versions in the requirements.txt. Now a package that pyre-check introduces a dependency conflict as it depends on a newer version of a package (pyre-extensions)
Bumping pyre-extensions which extend the typing module. Since it is non-breaking module (pyre-extensions), we could potentially unpin the versioning information and allow pyre-check to dictate the version. However, I could be wrong here.

Test Plan

Ran 102 tests in 5.983s

The frontend tests are failing however, but I strongly believe them to be un-related to this PR.

Fixes #89

Bump pyre-extensions version in requirements.txt
Since, these are extensions are for the typing module, I think
we can remove the version information as well.

Signed-off-by: Abishek V Ashok <abishekvashok@gmail.com>
@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Mar 3, 2023
@facebook-github-bot
Copy link
Contributor

@stroxler has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@@ -9,7 +9,7 @@ ipython<8.4.0
munch==2.5.0
prompt-toolkit==3.0.29
Pygments==2.12.0
pyre-extensions==0.0.27
pyre-extensions==0.0.30
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for the fix, looks good to me!

I'll try to get this landed today

@facebook-github-bot
Copy link
Contributor

@stroxler merged this pull request in 762a130.

@abishekvashok abishekvashok deleted the pyre-extensions-bump branch March 7, 2023 04:44
@abishekvashok
Copy link
Contributor Author

@stroxler maybe also consider:
we could potentially unpin the versioning information for pyre-extensions and allow pyre-check to dictate the version. However, I might be wrong here.

Cc: @0xedward who pinned versions :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error installing fb-sapp due to dependency issue
3 participants