Skip to content

Conversation

@mo-johnmo
Copy link
Contributor

@mo-johnmo mo-johnmo commented Jan 20, 2025

Closes #199.

PR creation checklist for the developer

  • Has <issue_number> above ☝️ been replaced with the issue number?
  • Has main been selected as the base branch?
  • Does the feature branch name follow the format <issue_number>_<short_description_of_feature>?
  • Does the text of the PR title exactly match with the text (not including the issue number) of the issue title?
  • Have appropriate reviewers been added to the PR (once it is ready for review)?
  • Has the PR been assigned to the developer(s)?
  • Have the same labels as on the issue (except for the good first issue label) been added to the PR?
  • Has the Climate Model Evaluation Workflow (CMEW) project been added to the PR?
  • Has the appropriate milestone been added to the PR?

Definition of Done for the developer

  • Does the change in this PR address the above issue / have all acceptance criteria been met?
  • Does the change in this PR follow the requirements in the wiki: Developer Guide (including copyrights)?
  • Have new tests related to the change been added?
  • Do all the GitHub workflow checks pass?
  • Do all the tests run locally and pass? (Note: the tests are not run by the GitHub workflow, see wiki: Run the tests locally)
  • Has the API documentation (e.g. docstrings in Python modules) related to the change been updated appropriately?
  • Has the user documentation (i.e. everything in the doc directory) related to the change been updated appropriately, including the Quick Start section?
  • Do the HTML pages render correctly? (See wiki: Build the documentation locally)

PR creation checklist for the reviewer

  • Has <issue_number> above ☝️ been replaced with the issue number?
  • Has main been selected as the base branch?
  • Does the feature branch name follow the format <issue_number>_<short_description_of_feature>?
  • Does the text of the PR title exactly match with the text (not including the issue number) of the issue title?
  • Have appropriate reviewers been added to the PR (once it is ready for review)?
  • Has the PR been assigned to the developer(s)?
  • Have the same labels as on the issue (except for the good first issue label) been added to the PR?
  • Has the Climate Model Evaluation Workflow (CMEW) project been added to the PR?
  • Has the appropriate milestone been added to the PR?

Definition of Done for the reviewer

  • Does the change in this PR address the above issue / have all acceptance criteria been met?
  • Does the change in this PR follow the requirements in the wiki: Developer Guide (including copyrights)?
  • Have new tests related to the change been added?
  • Do all the GitHub workflow checks pass?
  • Do all the tests run locally and pass? (Note: the tests are not run by the GitHub workflow, see wiki: Run the tests locally)
  • Has the API documentation (e.g. docstrings in Python modules) related to the change been updated appropriately?
  • Has the user documentation (i.e. everything in the doc directory) related to the change been updated appropriately, including the Quick Start section?
  • Do the HTML pages render correctly? (See wiki: Build the documentation locally)

SUPPORT.md added.
@mo-johnmo mo-johnmo added the community Community standards in CMEW label Jan 20, 2025
@mo-johnmo mo-johnmo self-assigned this Jan 20, 2025
@mo-johnmo mo-johnmo requested a review from ehogan January 20, 2025 15:34
@mo-johnmo mo-johnmo marked this pull request as draft January 20, 2025 15:34
@mo-johnmo mo-johnmo added this to the v0.1.0 milestone Jan 20, 2025
@mo-johnmo
Copy link
Contributor Author

This workflow will fail until PR #245 has been merged into main.
When that has been done, this branch will need to have main merged into it, or been rebased upon it.

@mo-johnmo mo-johnmo modified the milestones: v0.1.0, v0.2.0 Jan 20, 2025
* main:
  #245 Resolve workflow error possibly caused by previous version of pre-commit
@mo-johnmo mo-johnmo marked this pull request as ready for review January 21, 2025 09:51
@mo-johnmo
Copy link
Contributor Author

The main branch post PR #245 has been merged into this branch which is now ready for review.

@mo-johnmo mo-johnmo requested review from alistairsellar and removed request for ehogan January 21, 2025 11:54
Copy link
Collaborator

@alistairsellar alistairsellar left a comment

Choose a reason for hiding this comment

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

Hi John,

The support.md file is good. Only change I'm requesting is to remove the issue number from the PR title.

Thanks,
Alistair

@mo-johnmo mo-johnmo changed the title #199 add a support.md file Add a support.md file Jan 21, 2025
alistairsellar
alistairsellar previously approved these changes Jan 21, 2025
Copy link
Collaborator

@alistairsellar alistairsellar left a comment

Choose a reason for hiding this comment

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

Thanks John, all good!

@mo-johnmo mo-johnmo requested a review from ehogan January 21, 2025 14:27
Copy link
Member

@ehogan ehogan left a comment

Choose a reason for hiding this comment

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

Thanks @mo-johnmo 😊

Here are my review comments:

  • The "Does the text of the PR title exactly match with the text (not including the issue number) of the issue title?" PR checklist item has been checked, but the titles do not exactly match.
  • With regards to "Has the appropriate milestone been added to the PR?", the correct milestone to add for the work you are doing is v0.1.0.
  • The "Does the change in this PR address the above issue / have all acceptance criteria been met?" PR checklist item has been checked, but the issue states:
    • "Is there a way to consolidate this with the text in https://github.com/MetOffice/CMEW/blob/main/doc/source/support.rst?"
      • I was expecting changes to support.rst to point to the SUPPORT.md file or equivalent, such that there is a single source of truth / there wasn't conflicting information in different areas of the documentation.
    • "I just had a thought that perhaps we should use the Discussions as a way to get support, rather than a MO e-mail address that the community won't have access to." (emphasis added)
      • The MO e-mail address should not be used for support. Support should be solely via the repository via the Discussions.
  • Something that wasn't mentioned in the issue, but I'm thinking (after looking at https://github.com/MetOffice/CMEW/tree/199_add_support_md_file) that it might be a good idea is to include a support section in the README.md file that points to the SUPPORT.md file (it looks like GitHub hasn't automatically added it to the tabs next to the README tab).
  • The Documentation requirements states:
    • Limit all lines in markdown (.md) files and .rst files to a maximum of 79 characters.
    • Use reference style links to help limit all lines in markdown (.md) files to a maximum of 79 characters.
  • However, I will add under the first point in the Documentation requirements the following:
    • Do not break the content across multiple lines at 79 characters, but rather break them on semantic meaning (e.g. periods or commas). https://rhodesmill.org/brandon/2012/one-sentence-per-line/ provides more information. (This is a new thing I discovered recently and I think it is an awesome idea!) 🥳

@mo-johnmo mo-johnmo changed the title Add a support.md file Add a SUPPORT.md file Jan 29, 2025
@mo-johnmo mo-johnmo modified the milestones: v0.2.0, v0.1.0 Jan 29, 2025
@mo-johnmo mo-johnmo marked this pull request as draft January 30, 2025 10:51
alistairsellar
alistairsellar previously approved these changes Feb 5, 2025
Copy link
Collaborator

@alistairsellar alistairsellar left a comment

Choose a reason for hiding this comment

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

Great, thanks John!

@mo-johnmo mo-johnmo requested a review from ehogan February 5, 2025 14:20
Copy link
Member

@ehogan ehogan left a comment

Choose a reason for hiding this comment

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

It should be possible to use the MyST parser for markdown-rst-compatability. This was proposed in #220. We should consider whether to add this to the next community environment as part of #248.

The requirements for CMEW state to use the format #<issue_number>: <descriptive_commit_message> when writing commit messages (please note the use of :).

Please add single backticks around SUPPORT.md in the title of this PR so that it exactly matches the title of the issue.

@mo-johnmo mo-johnmo marked this pull request as draft February 10, 2025 10:29
@mo-johnmo mo-johnmo changed the title Add a SUPPORT.md file Add a SUPPORT.md file Feb 10, 2025
hypertext link.  While CMEW remains private the full URL will be copyable
but not clickable.
@mo-johnmo
Copy link
Contributor Author

PR title changed as requested in event.
A comment about using the MyST parser was added to issue #248.

@mo-johnmo mo-johnmo marked this pull request as ready for review February 10, 2025 12:50
alistairsellar
alistairsellar previously approved these changes Feb 10, 2025
@mo-johnmo mo-johnmo requested a review from ehogan February 10, 2025 14:14
…style to an automatic one

in order to stop the 'make html' failing.
Copy link
Member

@ehogan ehogan left a comment

Choose a reason for hiding this comment

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

Thanks @mo-johnmo 👍 Just one minor typo to fix, then I'll approve 🥳

Co-authored-by: Emma Hogan <ehogan@users.noreply.github.com>
@mo-johnmo mo-johnmo requested a review from ehogan February 25, 2025 09:44
Copy link
Member

@ehogan ehogan left a comment

Choose a reason for hiding this comment

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

Thanks @mo-johnmo! 🎉

@mo-johnmo mo-johnmo merged commit a5e9d54 into main Feb 25, 2025
3 checks passed
@mo-johnmo mo-johnmo deleted the 199_add_support_md_file branch February 25, 2025 16:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

community Community standards in CMEW

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add a SUPPORT.md file

4 participants