Pull Request Guide: Best Practice Improvements

[anilnatha]

Purpose

This PR addresses the addition of best practices to the pull request guide.

Proposed Changes

• [CHANGE] Added "REMOVED" to list of "proposed changes" items in the Pull Request template markdown file

Issues

• Resolves [Improve Existing Best Practice Guide]: Pull Request Process Improvement #186

Testing

• Provide some proof you've tested your changes
• Example: test results available at ...
• Example: tested on operating system ...

[riverma]

@anilnatha - I made some mods to your PR branch:

• A new template CODEOWNERS file, referenced in the guide and also added to slim-registry for future use from slim-cli
• Minor typo fixes and image updates to look better on a white screen background

I tested these changes successfully using a local deployment of the guide, via npm install; npm start.

LMK what you think!

[anilnatha]

@riverma Thank you for reviewing what I have initially proposed! The updates and additions you've made are great!

Do you think what we have captured here is sufficient for this PR? And if so, can we create a separate ticket to address the need of creating a best-practice guide(s) for repo set up to cover versioning and branch protection rules? Was there anything else that I missed that still needs to be covered that we spoke of?

[anilnatha]

I will also need to update the original post in this PR to cover the additional changes we have made since I originally opened the PR for collaboration.

[riverma]

Hey @anilnatha - I think its good to go! One thing we may want to update later is the link to the example CODEOWNERS file in the quickstart section, to point to an actual implementation of the CODEOWNERS that's valid and useful. Perhaps a future Unity example - we can update that later.

In terms of the content for versioning, I had a few ideas:

1. We could encourage folks to follow https://www.conventionalcommits.org/en/v1.0.0/ spec
2. The above could be enforced using a linter: https://commitlint.js.org/concepts/commit-conventions.html
3. No 1 and 2 are pre-reqs for setting up a whole automation solution, that updates the CHANGELOG, creates a release, and even publishes to third-party repos: https://github.com/googleapis/release-please or https://semantic-release.gitbook.io/semantic-release

We could push this to a new ticket?

[riverma]

Was there anything else that I missed that still needs to be covered that we spoke of?

I noticed we didn't have any recommendations for SonarQube - should we add a link to https://github.com/marketplace/actions/official-sonarqube-scan for that and later move the bulk of that content elsewhere to another guide?

Also - please push "Ready for review" when you feel we're good!

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[anilnatha]

Hey Rishi, I committed the sonarqube suggestion you made. Thanks!

As for the versioning issues that need to be pushed to a new ticket, Do you want me to create that ticket? I can help with that effort too if you wish.

[riverma]

LGTM! Thanks for this contribution @anilnatha - I think we're good to go. Approved!

Additionally, I want to thank you for taking the time to make this contribution to SLIM. Much appreciated, and additionally, it was a pleasure working with you on this contribution!

[riverma]

Nice 👍

[riverma]

Hey Rishi, I committed the sonarqube suggestion you made. Thanks!

As for the versioning issues that need to be pushed to a new ticket, Do you want me to create that ticket? I can help with that effort too if you wish.

Hey @anilnatha - I posted a comment suggesting an augmentation to an existing PR we have on releases. Shall we migrate the discussion and additional development work there?

[anilnatha]

@riverma I think that's a great idea, I'll review the other PR shortly.