-
Notifications
You must be signed in to change notification settings - Fork 4
fix: Install pip and pip-tools in upgrade script #29
Conversation
Need approval from the owning team as well. |
There was a problem hiding this 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.
requirements/common_constraints.txt
Outdated
@@ -0,0 +1,25 @@ | |||
# A central location for most common version constraints |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
There was a problem hiding this 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.
Oh yes, Thanks for the commit |
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.