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

docs: Update Interacting with github section #1222

Closed
wants to merge 2 commits into from

Conversation

YashPimple
Copy link
Member

@YashPimple YashPimple commented Apr 13, 2023

Related to #972
Fixes issue #997

Signed-off-by: Yash Pimple [email protected]

@github-actions github-actions bot added the documentation Improvements or additions to documentation label Apr 13, 2023
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@netlify
Copy link

netlify bot commented Apr 13, 2023

Deploy Preview for keptn-lifecycle-toolkit ready!

Name Link
🔨 Latest commit 9839cac
🔍 Latest deploy log https://app.netlify.com/sites/keptn-lifecycle-toolkit/deploys/64378728cde0ce0008f5a335
😎 Deploy Preview https://deploy-preview-1222--keptn-lifecycle-toolkit.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

* If you want to do the work on an issue,
include that information in your description of the issue
or in a comment to the issue.
## [Guidelines for contributing](https://github.com/keptn/lifecycle-toolkit/tree/main/docs/content/en/docs/guidelines-for-contributing.md)
Copy link
Member

Choose a reason for hiding this comment

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

please do not put links in headlines

Copy link
Contributor

@StackScribe StackScribe Apr 17, 2023

Choose a reason for hiding this comment

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

Agree with Simon. Also, the file to which you are linking is in the main docs directory rather than in a directory under keptn.sh/contribute/ . The "Guidelines for contributing" should be under the general subdirectory. To verify that you have a file in the proper location, look at the netlify preview and click the "Contributing" tab on the top toolbar to be sure that the content is showing up there.

Copy link
Contributor

Choose a reason for hiding this comment

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

Okay, you're getting closer but still a few big issues:

  • This PR is smaller than the last but still includes work associated with two issues -- github and guidelines. As a general rule, we should have one PR per issue. Moreover, if you look at the github issue, you will see that someone else already submitted a PR for that so we don't need a second one. It would, of course, be good for you to review the other PR. Doing these small, targeted PRs makes it easier to review (some people can review the github piece; others can review the guidelines piece. This is kind of a core practice for both Agile development and open source and it really is tough to get used to. However, once you get used to it, I think you will see its advantages.
  • Your source is located in the docs/content/en/docs directory rather than in the docs/content/en/contribute directory. You should be building the docs locally and checking them then check them again on netlify after you submit the PR. I do not think that the contents of this PR show up on either build. You should see it when you click on the "Contributing" tab on the top of the page.

So let's turn this into a PR that is just for the "guidelines" with the source in the proper directory so we can view it. Also feel free to modify the content and add content. You are actually the target audience for much of this material so see if it makes sense and if it is complete and make any improvements you can.

@YashPimple YashPimple closed this May 11, 2023
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