Updated section "backward compatibility" in contributing.rst #1918
Conversation
Codecov Report
@@ Coverage Diff @@
## main #1918 +/- ##
=======================================
Coverage 92.18% 92.18%
=======================================
Files 234 234
Lines 12213 12213
=======================================
Hits 11259 11259
Misses 954 954 Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
|
@alistairsellar we're starting to get our ducks in line for the impending 2.8 release, mate - this one is marked as M2.8 but it's still a draft, you reckon you can make it RfR soon please? Cheers 🍺 |
Include instruction to tag the core development team and allow time for objections before merging a backward incompatible change
|
OK @valeriupredoi is now ready after one final change. As per @axel-lauer's description, this PR links with Tool doc changes (ESMValGroup/ESMValTool#2879), and so doesn't necessarily make sense on its own (it possibly introduces broken links without the linked PR). Also, not sure if the new references should be of the form |
|
many thanks @alistairsellar 🍺 A few pointy points:
|
|
Hi all, the start of the release process is approaching (next week). It would be great to have this in. Since @axel-lauer started the PR, GitHub does let the author to be the reviewer. Axel, feel free to comment here if the PR looks good to you. @alistairsellar, I'd be happy to take a final look at this if you wish. Feel free to assign me and anyone who could/should have a look at this PR. Thanks 👍 |
Aiming to fix build warnings for these links (WARNING: Mismatch: both interpreted text role prefix and reference suffix)
|
Thanks @valeriupredoi. I've now looked into the docs build error and can't figure the cause. Please could you, @axel-lauer @remi-kazeroni advise? The error message is very generic and doesn't tip me off about the cause: Also two warnings in the output about cross-references - could these be the underlying cause of the failure?: If so, they may arise from the fact that the doc does not yet exist in the esmvaltool documentation? They will resolve correctly when ESMValGroup/ESMValTool#2879 is merged. |
|
cheers @alistairsellar - it's those two warnings, the build is set to fail on warnings - would you be ok to fix those pls 🍺 |
|
Hi @valeriupredoi, I think (though not certain and welcome an alternative hint) that the warnings are because the referenced document does not yet exist in the esmvaltool documentation. If I'm right, then the only way to fix them is to merge ESMValGroup/ESMValTool#2879. |
|
@alistairsellar it looks that way indeed - is Tool/2879 ready to be merged? I just dumped |
So that backward compatibility policy can link to the changelog as a whole, not just specific version. This label makes the changelog consistent with its equivalent in the ESMValTool docs.
|
@valeriupredoi, ESMValGroup/ESMValTool#2879 has now been merged, and build now succeeding (after a final fix to links). So I think this PR is ready to merge. |
|
... pending review. I've tagged @remi-kazeroni following his kind offer above. |
|
great stuff, cheers @alistairsellar 🍺 Two things pls: could you fix the PR description (boxes need a good sweep), and merge |
|
Thanks @valeriupredoi. Merged in main. Sorry, I need you to explain "boxes need a sweep" in words of one syllable. Should I remove the checks that are not relevant, or tick the boxes that I believe are done (not sure whether developer or reviewer is supposed to do that), or both, or perhaps something else? |
|
hahah let me do that for you now, you have a look, and next time you hear "clean up boxes" (in this context, not when your wife asks you to do that at home), you'll know what that means 😁 |
There was a problem hiding this comment.
cheers muchly @alistairsellar 🍺 @remi-kazeroni @bouweandela you guys have a wee bit of time have a last look at this and merge, plese?
EDIT: my apologies @alistairsellar - just noticed this was @axel-lauer 's PR not yours, sorry to have put you through the paces to clean up PR descriptions - it's the PR OP that needs to do that 😁
There was a problem hiding this comment.
Thanks for taking care of this topic @alistairsellar and @axel-lauer! This looks good to me. I just have 2 very minor suggestions for improvement.
Good to have our backward compatibility policy fully documented. Let's make good use of that to draft the Changelog for the upcoming release 👍
Co-authored-by: Rémi Kazeroni <70641264+remi-kazeroni@users.noreply.github.com>
Co-authored-by: Rémi Kazeroni <70641264+remi-kazeroni@users.noreply.github.com>
Mirroring what has been added to equivalent section in Tool documentation
|
Righteeho, I've resolved Bouwe's request, and I think that Remi's requests have been folded in. So I think all we need is for @remi-kazeroni to mark as approved and then we're good to merge... |
|
cheers, bud @alistairsellar 🍺 |
Description
This PR adds a reference to the new "backward compatibility policy" to the documentation (contributing.rst, Backward compatibility: https://docs.esmvaltool.org/projects/ESMValCore/en/latest/contributing.html#backward-compatibility).
This PR should be merged together with PR ESMValGroup/ESMValTool#2879, which introduces new backward compatibility policy to ESMValTool documentation.
Closes discussion: ESMValGroup/Community#7
Link to documentation
Checklist
It is the responsibility of the author to make sure the pull request is ready to review. The icons indicate whether the item will be subject to the 🛠 Technical or 🧪 Scientific review.