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

Add changelog for v2.6.0rc1 #1633

Merged
merged 2 commits into from
Jun 13, 2022
Merged

Add changelog for v2.6.0rc1 #1633

merged 2 commits into from
Jun 13, 2022

Conversation

sloosvel
Copy link
Contributor

@sloosvel sloosvel commented Jun 13, 2022

Description

This PR adds the changelog for the first release candidate of v2.6

@codecov
Copy link

codecov bot commented Jun 13, 2022

Codecov Report

Merging #1633 (ee237a2) into main (46f9c01) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #1633   +/-   ##
=======================================
  Coverage   91.38%   91.38%           
=======================================
  Files         204      204           
  Lines       11173    11173           
=======================================
  Hits        10211    10211           
  Misses        962      962           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 46f9c01...ee237a2. Read the comment docs.

@sloosvel sloosvel merged commit 71ac7ee into main Jun 13, 2022
@sloosvel sloosvel deleted the changelog_26rc1 branch June 13, 2022 14:28
@zklaus
Copy link

zklaus commented Jun 14, 2022

This PR is not ready for review, much less for approval, and less still for merging since the checklist is not addressed.

@schlunma
Copy link
Contributor

I disagree. I (as one of the previous release managers who is responsible for this kind of PRs) think this PR is fine to be merged.

The title is fine and the missing label will be noticed (at the latest) by Saskia when she creates the new changelog (I admit that I didn't check that, but this is something that can be adapted later).

Also, I was not aware that we have a review process of the review in place.

@schlunma schlunma added the documentation Improvements or additions to documentation label Jun 14, 2022
@zklaus
Copy link

zklaus commented Jun 14, 2022

While the verbiage for the ESMValTool is more explicit (

[The reviewer] looks at the list of files that were changed in the pull request and checks that all relevant checkboxes from the checklist in the pull request template have been added and ticked.

), the one for the core seems at least suggestive to me:

Please try to do everything on the list before requesting a review.

I think it's clear that no PR should be merged with unticked open items in the checklist; all items should be either ticked or removed. Ensuring that PRs for which the core team is responsible are in tip-top shape will encourage everyone else to stick to good practices too.

I don't think that "we can always fix this later" is a good argument to disregard the checklist. After all, the purpose of that is to streamline things and not to create more work down the line.

As for reviewing the review, I think everyone can comment where there seems to be a discrepancy between the published procedure and the procedure followed. As core team, we have perhaps a duty to do so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants