#699/first impression

[jesperolsson-se]

Description

Lowers the barrier to contribution by focusing the README.

Issues Resolved

699

Check List

• [ N/A ] New functionality includes testing.
  • [ N/A ] All tests pass
• [ N/A ] New functionality has been documented.
  • [ N/A ] New functionality has javadoc added
• [ X ] Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

[opensearch-ci-bot]

✅   DCO Check Passed edfcfdf88b4b97431b138494549f153276c60603

[opensearch-ci-bot]

✅   Gradle Wrapper Validation success edfcfdf88b4b97431b138494549f153276c60603

[opensearch-ci-bot]

✅   Gradle Precommit success edfcfdf88b4b97431b138494549f153276c60603

[dblock]

Left some comments.

[dblock]

Commit into this repo.

[jesperolsson-se]

I'm not sure I understand. Could you please clarify?

[AmiStrn]

I think the comment is suggesting the svg file be committed directly to this repo rather than referencing another one to retrieve it.

[dblock]

Yup.

[jesperolsson-se]

That would A) duplicate the image; and B) make it impossible to know the source of truth. Would you please state what's the problem with a reference, and if that could be resolved by a less volatile location e.g., https://www.opensearch.org/logo.svg ?

[dblock]

That external reference to docs has no idea it's being used and is not a stable URL, it will break. IMO anything external will break, but it's not important enough for me, so if you want to use a less volatile location, e.g. https://opensearch.org/brand.html, that's A-OK.

[stockholmux]

Notes on the positioning as an alternative

[opensearch-ci-bot]

✅   DCO Check Passed ef951cbd71cdea5f4d732901c4c0b40699f2f2a2

[opensearch-ci-bot]

✅   Gradle Wrapper Validation success ef951cbd71cdea5f4d732901c4c0b40699f2f2a2

[opensearch-ci-bot]

✅   Gradle Precommit success ef951cbd71cdea5f4d732901c4c0b40699f2f2a2

[dblock]

LGTM, can I nitpick and ask you to get rid of the text wrap at 80 columns? Markdown doesn't care and the diffs are easier to read without it. You also need to rebase since looks like more README changes have been made since.

Thanks for hanging in here!

[jesperolsson-se]

@dblock

I appear to be unable to reply directly to your "nitpick" comment (perhaps it wasn't marked as a review comment?), so please find my answer below. Also, I'd be grateful if you could resolve the earlier conversations that you're satisfied with.

Nitpicking is more than fine: I often find that they are expressions for deeper (tacit) concerns 🙂 In this case, however, would you mind clarifying your reasoning - perhaps even in a project style guide, so that the same discussion won't have to take place in the future? At some level, I guess, I don't see why we should forego wrapping just because we're dealing with a README.

My perception is that a 80 or 100 columns restriction improves diff readability in most relevant situations, e.g., editorial changes don't get swamped by the rest of the paragraph. Are there any particular scenarios in which you find the opposite to be true? Also, do you diff on entire PRs or on individual commits?

I don't think I understand your comment about markdown not caring - neither do most programming languages, but the popular style guides still enforce wrapping. Could you please clarify? (I wrap because I find such source files to be easier to work with, not because I'm misguided about markdown rendition.)

Thank you for considering my patience: It makes me feel appreciated. For the most part, I'm fine with taking the time to sort out all worries, especially when I believe doing so benefits the project of the individuals 🙂 For example, should you decide to raise a ticket and send up a PR for a minimal style guide, I'm happy to wait for it (that way, you won't have to duplicate your text).

[dblock]

I decided I'd write this up here :)

[dblock]

@jesperolsson-se can you please rebase this, there's a merge conflict.

I don't care about the 80 column thing for this PR.

[opensearch-ci-bot]

✅   DCO Check Passed 0192c28

[opensearch-ci-bot]

✅   Gradle Wrapper Validation success 0192c28

[opensearch-ci-bot]

✅   Gradle Precommit success 0192c28

[jesperolsson-se]

@dblock

It pleases me to hear that our discussion inspired you to write, and I hope that others find it helpful. While I don't believe you meant any harm, I would like to request that you ask for permission next time.

Of course, anything we write here becomes public, but that's not really my point. Perhaps it's a cultural difference (I really can't tell 🙂) but, to me, asking for consent prior to publication is a sign of respect for the individual. It's difficult to express this notion (and even more so in text), and perhaps it isn't very important to do so because I'll gladly give you my consent a posteriori.

Regarding this PR, I've now rebased (it seems like you were the author whom I had conflicts with). Most of the conflicts related to paragraphs that I was removing or rewriting, and a couple were style differences for which we have no guide. The end result is pretty much identical to what you've reviewed thus far in this PR.

[dblock]

@dblock

It pleases me to hear that our discussion inspired you to write, and I hope that others find it helpful. While I don't believe you meant any harm, I would like to request that you ask for permission next time.

Of course, anything we write here becomes public, but that's not really my point. Perhaps it's a cultural difference (I really can't tell 🙂) but, to me, asking for consent prior to publication is a sign of respect for the individual. It's difficult to express this notion (and even more so in text), and perhaps it isn't very important to do so because I'll gladly give you my consent a posteriori.

I am sorry if I did something wrong, that was definitely no intent to do so. I wrote why I prefer not to wrap text in markdown, which seems like not something I should seek permission for. I do not mention you in the post, and link to your well-written publicly available comment about the matter. What permission did I need to seek?

Regarding this PR, I've now rebased (it seems like you were the author whom I had conflicts with). Most of the conflicts related to paragraphs that I was removing or rewriting, and a couple were style differences for which we have no guide. The end result is pretty much identical to what you've reviewed thus far in this PR.

Thanks

[dblock]

start gradle check

[opensearch-ci-bot]

✅   Gradle Check success 0192c28
Log 230

Reports 230