Skip to content
This repository has been archived by the owner on May 29, 2024. It is now read-only.

fix: Install pip and pip-tools in upgrade script #29

Merged
merged 4 commits into from
Aug 11, 2022

Conversation

Jawayria
Copy link
Contributor

Updated the upgrade target script to check the compatibility of upgraded pip and pip-tools versions. For reference, look at this openedx/edx-repo-health#271 for new check added in edx-repo-health.

@UsamaSadiq
Copy link

Need approval from the owning team as well.

Copy link

@feanil feanil left a comment

Choose a reason for hiding this comment

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

One question, it seems like there shouldn't be a common_constraints.txt file in each repo since that would defeat the purpose of the common file. The comment in that file also suggests this.

@@ -0,0 +1,25 @@
# A central location for most common version constraints
Copy link

Choose a reason for hiding this comment

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

@Jawayria should this file be a copy or should it just be linking to the central constraints file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We mostly do this when we have to override some constraint but it isn't needed here so updated the PR

@feanil feanil self-assigned this Aug 10, 2022
@Jawayria Jawayria requested a review from feanil August 11, 2022 13:11
Copy link

@feanil feanil left a comment

Choose a reason for hiding this comment

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

Looks like you had forgotten to delete the old requirements.txt file, I just added a commit for that onto this PR so now it's good to go.

@feanil feanil merged commit 3c1cff0 into aximcollaborative:main Aug 11, 2022
@Jawayria
Copy link
Contributor Author

Looks like you had forgotten to delete the old requirements.txt file, I just added a commit for that onto this PR so now it's good to go.

Oh yes, Thanks for the commit

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants