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

Readme Get Started Section #4465

Merged
merged 4 commits into from
Aug 23, 2023
Merged

Readme Get Started Section #4465

merged 4 commits into from
Aug 23, 2023

Conversation

shanecmiller23
Copy link
Contributor

@shanecmiller23 shanecmiller23 commented Aug 7, 2023

Description of your changes

Add section for Get Started to the Readme. Right now there is no direction for brand new users who stumble upon the GitHub Readme.

I have:

  • Read and followed Crossplane's contribution process.
  • Added or updated unit and E2E tests for my change.
  • Run make reviewable to ensure this PR is ready for review.
  • Added backport release-x.y labels to auto-backport this PR if necessary.
  • Opened a PR updating the docs, if necessary.

Add section for Getting Started

Signed-off-by: Shane Miller <60621688+shanecmiller23@users.noreply.github.com>
@shanecmiller23 shanecmiller23 requested a review from a team as a code owner August 7, 2023 21:35
README.md Outdated
@@ -11,6 +11,10 @@ control of the schema of the declarative API it offers.

Crossplane is a [Cloud Native Computing Foundation][cncf] project.

## Getting Started

Crossplane's [Getting Started Documentation](https://docs.crossplane.io/v1.13/getting-started/) covers Crossplane installation and cloud provider quickstarts.
Copy link
Member

Choose a reason for hiding this comment

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

Is there some kind of latest link we can use? Just to remove one more place we'll need to keep the URL fresh as new versions are released.

Other than that, LGTM. Could you wrap at 80 chars to match the rest of the doc though?

Copy link
Contributor

Choose a reason for hiding this comment

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

Use /latest instead of the version for an evergreen link

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated

fix version linking

Signed-off-by: Shane Miller <60621688+shanecmiller23@users.noreply.github.com>
README.md Outdated
@@ -11,6 +11,10 @@ control of the schema of the declarative API it offers.

Crossplane is a [Cloud Native Computing Foundation][cncf] project.

## Get Started

Crossplane's [Getting Started Documentation](https://docs.crossplane.io/latest/getting-started/) covers Crossplane installation and cloud provider quickstarts.
Copy link
Member

Choose a reason for hiding this comment

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

Link looks good to me now @shanecmiller23, but can you still wrap this new content to 80 chars so it's consistent with the rest of the file? 😇

Feel free to also squash down to a single commit to keep the history clean 🤓

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jbw976 updated. I don't know how to squash down to a single commit, would that just entail opening a new PR?

Copy link
Member

Choose a reason for hiding this comment

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

no sir, you can squash using a git interactive rebase, but let's not worry about that 🤓 - we can also let github do it for us when we merge the PR.

But I'm still not seeing this line wrapped at 80 chars, did you push a commit for that? See the screenshot below where the new line here goes much wider than the other lines in the doc? do you see differently? 🤔
Screenshot 2023-08-18 at 9 30 24 AM

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jbw976 it does go longer in that screenshot, but that's only because it includes a full URL that will be the hidden destination for the link text. So in the actual readme that URL part won't show

Copy link
Member

Choose a reason for hiding this comment

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

yep agreed that it does get wrapped when it's fully rendered @shanecmiller23! what i'm getting at here though is that we want to keep the raw markdown clean and having a consistent style. So in the raw markdown we like to do the following:

So putting that together, I'd expect your contribution here to look like this in the raw markdown:

## Get Started

Crossplane's [Getting Started Docs] cover install and cloud provider
quickstarts.

Then at the bottom of the doc, you'd add this new reference style link value:

[Getting Started Docs]: https://docs.crossplane.io/latest/getting-started/

Does that make sense? I'm sorry for the back and forth here, happy to DM or meet for higher bandwidth discussion if there's any lingering questions. Thank you @shanecmiller23! 🙇

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jbw976 updated per your comments! apologies on my misunderstanding

revise to be under 80 characters

Signed-off-by: Shane Miller <60621688+shanecmiller23@users.noreply.github.com>
fix per Jared's feedback

Signed-off-by: Shane Miller <60621688+shanecmiller23@users.noreply.github.com>
@shanecmiller23 shanecmiller23 changed the title Readme Getting Started Section Readme Get Started Section Aug 23, 2023
Copy link
Member

@jbw976 jbw976 left a comment

Choose a reason for hiding this comment

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

Thanks @shanecmiller23! appreciate this contribution and iterating on the style preferences ✅

@jbw976 jbw976 merged commit 8526b49 into crossplane:master Aug 23, 2023
16 checks passed
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.

None yet

4 participants