Skip to content

Conversation

lovisaberggren
Copy link
Collaborator

Proposed changes

Updating the README for spectral changes to guide MMS/openapi maintainers depending on the change to the linting/OAS and changes needed to propagate to MMS.

_Jira ticket: CLOUDP-278403

Checklist

  • I have signed the MongoDB CLA
  • I have added tests that prove my fix is effective or that my feature works

@lovisaberggren lovisaberggren marked this pull request as ready for review October 15, 2024 11:10
@lovisaberggren lovisaberggren requested a review from a team as a code owner October 15, 2024 11:10
Comment on lines 26 to 36
1. Open a PR in MMS with the OAS changes and updating the MMS `.spectral.yaml` with the new/changed rules
2. If the current `mongodb/openapi` spectral rules will violate the OAS changes, open a PR in `mongodb/openapi` and update/disable any rules that will fail
3. Validate that the Spectral lint passes in `mongodb/openapi` and in MMS
4. Review and merge both PRs
5. Wait for the next release when the published OAS is updated
6. Open a `mongodb/openapi` PR updating the linting `spectral-lint.yaml` with the linting changes initially added to MMS
7. Validate that all tests pass
8. Review and merge the PR
9. Open a PR in MMS, updating the commit SHA of the imported spectral file, and removing any rules that were added to `mongodb/openapi`
10. Validate all tests pass
11. Review and merge the MMS PR
Copy link
Collaborator

Choose a reason for hiding this comment

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

I am not sure I understood these steps. In the scenario the new rule breaks the release, we need to update the MMS OAS (or the other services OASes depending on where is the error), backport the fix to the vbranch and wait 1 days that the service is deployed in all the env (dev, qa, staging and prod). once that happens, you can open a PR agains mongodb/openapi and update the spectral rule

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yup that's pretty much it in a nutshell. It was a bit tricky to explain in clear steps, perhaps the contents right now are too verbose. Would you like me to try to make it a bit shorter and simpler? Or is there anything in the steps now that you wouldn't agree with?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I agree with @wtrocki on the fact that this is a public repo and we should not probably talk about MMS. I think you could add a link in the readme to the internal wiki: https://wiki.corp.mongodb.com/pages/viewpage.action?pageId=306447071 and move these steps there. You can even add the link to the PR template to make sure the developer had a look to the wiki before making changes to spectral.yml. Up to you if you want to add the wiki to the readme or to the template file

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Alright sounds good, thanks!

1. Open a `mongodb/openapi` PR with the changes to `tools/spectral/.spectral.yaml`
2. Validate that the new Spectral lint checks pass
3. Review and merge the PR
4. Open a PR in MMS, updating the commit SHA of the imported spectral file
Copy link
Member

@wtrocki wtrocki Oct 15, 2024

Choose a reason for hiding this comment

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

FYI: For public repositories, we keep internal documentation in wikis. It allows us to put links and all information without being restricted in any way.

Fine to keep it here but feel free to move it to wiki if that helps your use case.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, though I think the current content is alright to keep here (also suggested by @blva to update this README). @andreaangiolillo Are you good with keeping the contents here, or would you prefer an internal wiki linked through a golink instead?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Went with moving the contents to the wiki and keeping the README lighter for public contributions, RFAL!

Copy link
Collaborator

@andreaangiolillo andreaangiolillo left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for all the changes!

@lovisaberggren lovisaberggren merged commit ab5e769 into main Oct 15, 2024
10 checks passed
@lovisaberggren lovisaberggren deleted the CLOUDP-278403 branch October 15, 2024 13:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants