openedx / open-edx-proposals

Proposals for Open edX architecture, best practices and processes
http://open-edx-proposals.readthedocs.io/
Other
43 stars 32 forks source link

Update OEP-18 Python Dependency Management with more constraint tips #332

Open robrap opened 2 years ago

robrap commented 2 years ago

In order to help with unpinning pinned constraints, we would like to propose the following updates when pinning constraints:

  1. Include the pinning date in the comment for the pin. This will make it simpler to know how out-of-date we are.
  2. Create a github issue for unpinning the constraint.
sarina commented 2 months ago

@robrap @feanil could you weigh in on whether this would be a helpful idea?

robrap commented 2 months ago

I believe this would be helpful, and should be relatively simple. I captured it as an issue, rather than opening a PR, because I just didn't have the time to deal with the edits and see it through.

sarina commented 1 month ago

Makes sense, thanks. I'd like to see what @feanil and/or other members of Maintainers WG feels about this - I don't think I'm the best person to propose this change, as I am not deeply maintaining code repos, but I can help this process move forward if the Maintainers WG wants to push it forward.

kdmccormick commented 1 month ago

Big +1 to the GitHub issue idea. Constraints are always a problem during upgrades, so having documentation and accountability for any constraint seems like a solid best practice.

My instinct would be to put the date in the GitHub issue rather than the comment, but that's not a strongly-held opinion. I'd support the change either way.

feanil commented 1 month ago

I agree that it would be useful to have a github issue for any new constraint that gets added. I think I could take or leave aspirational dates but the details about why the constraint was needed are super valuable.

robrap commented 1 month ago

My instinct would be to put the date in the GitHub issue rather than the comment, but that's not a strongly-held opinion. I'd support the change either way.

My thinking on the dates was simply that when looking over the constraints files, seeing that a constraint was added 5 years ago might stand-out and might get some action. Git blame is more for when you know what you are looking for, and dates on issues requires click-through. Anyway - just a thought.

I think I could take or leave aspirational dates

There should be nothing aspirational about the date. It is simply the date on which the constraint was pinned/added.

sarina commented 1 month ago

With three votes, but from people who've thought about this a lot, I think this is a good recommendation to move forward with.

@feanil I agree with Robert there's nothing in the issue description about an aspirational date of removal. This is just about documenting when the constraint was added, by both a comment and an associated GitHub issue.

I'm marking this as the Help Wanted label for anyone in the community to pick up, but I will try to draw down open issues as well over the next months.

robrap commented 1 month ago

Note that equally important to documenting this would be putting it into practice in edx-platform and the common constraints file. We could chip away at this, but it might help to make issues for these two repos.