Minor updates #5889

richsalz · 2018-04-05T20:25:24Z

Not planning to backport (doc-nits is only in master) unless there's pressure.

levitte · 2018-04-05T20:30:36Z

CONTRIBUTING

 ------------------------------------

 (Please visit https://www.openssl.org/community/getting-started.html for
 other ideas about how to contribute.)

-Development is coordinated on the openssl-dev mailing list (see the
+Development is coordinated on the openssl-project mailing list (see the


No. openssl-project is not intended to be a new openssl-dev. We should say that we're coordinating development on github.

levitte · 2018-04-05T20:30:47Z

CONTRIBUTING

 above link or https://mta.openssl.org for information on subscribing).
 If you are unsure as to whether a feature will be useful for the general
-OpenSSL community you might want to discuss it on the openssl-dev mailing
+OpenSSL community you might want to discuss it on the openssl-project mailing


No. You raise an issue on github.

levitte · 2018-04-05T20:31:42Z

CONTRIBUTING

 list first.  Someone may be already working on the same thing or there
 may be a good reason as to why that feature isn't implemented.

 To submit a patch, make a pull request on GitHub.  If you think the patch
-could use feedback from the community, please start a thread on openssl-dev
+could use feedback from the community, please start a thread on openssl-project


No, just submit a PR on github, discussion will happen there.

levitte · 2018-04-05T20:33:53Z

I would also very much like to see whatever changes we end up with backported to 1.1.0, as much as is appropriate (yes, I know, doc-nits is master only, but the rest is the same for all branches)

t-j-h · 2018-04-05T22:56:38Z

As I noted in the other PR - we need to be clear to point what used to go to openssl-dev off to github as per OMC decisions. openssl-project is not a replacement for openssl-dev - it has an entirely different purpose.

And the elimination of RT references should be backported for active release (1.0.2, 1.1.0) ...

mspncp · 2018-04-06T07:02:16Z

CONTRIBUTING

@@ -1,26 +1,26 @@
-HOW TO CONTRIBUTE PATCHES TO OpenSSL
+HOW TO CONTRIBUTE TO PATCHES OpenSSL


Did you change this line intentionally? This does not sound like correct english to me.

Maybe you intended to write "HOW TO CONTRIBUTE TO OpenSSL"?

mspncp · 2018-04-06T07:05:05Z

CONTRIBUTING


-    1. Anything other than trivial contributions will require a contributor
+To make it easier to review and accept our pull request, please follow these


'our pull request' -> 'your pull request'

mspncp · 2018-04-06T07:08:23Z

CONTRIBUTING

+To make it easier to review and accept our pull request, please follow these
+guidelines:
+
+    1. Anything other than a trivial contribution requires a contributor
    licensing agreement, giving us permission to use your code. See


It's better to use the formulation of cla.html, which calls it a "Contributor License Agreement (CLA)".

mspncp · 2018-04-06T07:09:18Z

CONTRIBUTING

@@ -1,26 +1,26 @@
-HOW TO CONTRIBUTE PATCHES TO OpenSSL
+HOW TO CONTRIBUTE TO PATCHES OpenSSL


Maybe you intended to write "HOW TO CONTRIBUTE TO OpenSSL"?

mspncp · 2018-04-06T07:12:45Z

CONTRIBUTING

-    documentation. Please look at the "pod" files in doc/man[1357]
-    for examples of our style.
+    documentation. Please look at the "pod" files in doc/man[1357] for
+    examples of our style. Do "make doc-nits" to make sure that your


Maybe replace "Do" -> "Run" ?

levitte

This looks good to me.

I hope you will do a backport for 1.1.0 as well

mspncp · 2018-04-06T12:31:44Z

CONTRIBUTING

+guidelines:
+
+    1. Anything other than a trivial contribution requires a Contributor
+    Licensing Agreement (CLA), giving us permission to use your code. See


Nit: the official wording is „License“, not „Licensing“, see the cla pdf.

richsalz · 2018-04-06T12:39:45Z

I'll wait until @t-j-h has a chance to look before pushing. I will cherry-pick, by just copying the file over to each release (and deleting the doc-nits sentence for the non-master branches).

mspncp

LGTM now. But I‘ll abstain and leave it for an OMC member to approve.

richsalz · 2018-04-06T12:48:53Z

Thanks for your feedback. You don't have to abstain :) Tim had made excellent points, too, and I want to make sure this addresses them before I merge.

FdaSilvaYY · 2018-04-06T20:42:37Z

CONTRIBUTING

-To submit a patch, make a pull request on GitHub.  If you think the patch
-could use feedback from the community, please start a thread on openssl-dev
-to discuss it.
+To request new features or report bugs, please open an issue.


... an issue on GitHub.

FdaSilvaYY · 2018-04-06T20:44:37Z

CONTRIBUTING


-Having addressed the following items before the PR will help make the
-acceptance and review process faster:
+To submit a patch, make a pull request on GitHub.  If you are thinking of


"To submit a patch, please open a pull request on GitHub..."

There is some steps before opening a PR that can be mentionned, like cloning the OpenSSL repository, create our work branch, push the change onto it, and them create a PR with this edited branch.

I took the wording change, but this is not the place for a GitHub tutorial, so not going to do that. Having that kind of doc, probably on our website, makes sense.

FdaSilvaYY · 2018-04-06T20:52:36Z

CONTRIBUTING

-    platforms: try to ensure you only use portable features.
-    Clean builds via Travis and AppVeyor are expected, and done whenever
-    a PR is created or updated.
+    platforms: try to ensure you only use portable features.  Clean builds


"Clean builds..." deserve a paragraph of its own, with some word on creating/opening a personal Travis/AppVeyor account ( very quick stepo using own GitHub login account) , sot anyone can push and test there changes without opening a PR.

Again, I think this should be a separate doc on the website.

mspncp · 2018-04-06T22:28:50Z

@FdaSilvaYY, you propose a lot of good ideas here, but some of them go beyond the scope of a simple CONTRIBUTING text file. For example, it can't be purpose of it to provide a complete GitHub tutorial. There are are a lot of good tutorials out there, not only on GitHub.

I agree that it would be a good thing to have a place were OpenSSL specific GitHub matters like the basic workflow and the build servers are explained. But a plain text format is not the ideal medium for doing that. The Wiki would be a more suitable location.

An even better and more visible solution (than a link to the wiki) is the possibility to create a welcome page for the project. I happened to see a nice one just recently on the google/tink project page. If you scroll down a little, you see it. The source file, README.md is part of the repository and written in github flavoured markdown. From the welcome page, one could link into further useful documentation related to OpenSSL on GitHub, which could be located in docs/github as markdown files. There you would have a lot of possibilities to follow your ideas...

richsalz · 2018-04-07T14:31:57Z

I know @t-j-h is busy this week with RSA, so I'll merge in a couple of days anyway.

t-j-h

Looks good!

Reviewed-by: Richard Levitte <levitte@openssl.org> Reviewed-by: Tim Hudson <tjh@openssl.org> (Merged from #5889)

Reviewed-by: Richard Levitte <levitte@openssl.org> Reviewed-by: Tim Hudson <tjh@openssl.org> (Merged from #5889) (cherry picked from commit 2876872)

richsalz · 2018-04-07T17:51:35Z

Merged to all three branches (sans the doc-nits sentence for non-master); thanks everyone for the feedback.

richsalz added branch: master Merge to master branch approval: review pending This pull request needs review by a committer labels Apr 5, 2018

richsalz self-assigned this Apr 5, 2018

levitte requested changes Apr 5, 2018

View reviewed changes

mspncp reviewed Apr 6, 2018

View reviewed changes

levitte approved these changes Apr 6, 2018

View reviewed changes

levitte added approval: done This pull request has the required number of approvals and removed approval: review pending This pull request needs review by a committer labels Apr 6, 2018

mspncp reviewed Apr 6, 2018

View reviewed changes

Rich Salz added 5 commits April 6, 2018 09:12

Minor updates

8c346c8

Address review feedback

f7e952e

Fix word-o

666e873

Matthias's feedback

37613a5

Licensing->License

76f2b7d

levitte approved these changes Apr 6, 2018

View reviewed changes

FdaSilvaYY reviewed Apr 6, 2018

View reviewed changes

Feedback from FdaSilvaYY

38125b2

t-j-h approved these changes Apr 7, 2018

View reviewed changes

levitte pushed a commit that referenced this pull request Apr 7, 2018

Updated to CONTRIBUTING to reflect GitHub, etc.

2876872

Reviewed-by: Richard Levitte <levitte@openssl.org> Reviewed-by: Tim Hudson <tjh@openssl.org> (Merged from #5889)

levitte pushed a commit that referenced this pull request Apr 7, 2018

Updated to CONTRIBUTING to reflect GitHub, etc.

facd09a

Reviewed-by: Richard Levitte <levitte@openssl.org> Reviewed-by: Tim Hudson <tjh@openssl.org> (Merged from #5889) (cherry picked from commit 2876872)

levitte pushed a commit that referenced this pull request Apr 7, 2018

Updated to CONTRIBUTING to reflect GitHub, etc.

06196e7

Reviewed-by: Richard Levitte <levitte@openssl.org> Reviewed-by: Tim Hudson <tjh@openssl.org> (Merged from #5889) (cherry picked from commit 2876872)

richsalz closed this Apr 7, 2018

richsalz deleted the update-contributing branch April 24, 2018 15:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Minor updates #5889

Minor updates #5889

richsalz commented Apr 5, 2018

levitte Apr 5, 2018

levitte Apr 5, 2018

levitte Apr 5, 2018

levitte commented Apr 5, 2018

t-j-h commented Apr 5, 2018

mspncp Apr 6, 2018

mspncp Apr 6, 2018

mspncp Apr 6, 2018

mspncp Apr 6, 2018

mspncp Apr 6, 2018

mspncp Apr 6, 2018

levitte left a comment

mspncp Apr 6, 2018

richsalz Apr 6, 2018

richsalz commented Apr 6, 2018

mspncp left a comment

richsalz commented Apr 6, 2018

FdaSilvaYY Apr 6, 2018

richsalz Apr 7, 2018

FdaSilvaYY Apr 6, 2018

FdaSilvaYY Apr 6, 2018

richsalz Apr 7, 2018

FdaSilvaYY Apr 6, 2018

richsalz Apr 7, 2018

mspncp commented Apr 6, 2018 •

edited

richsalz commented Apr 7, 2018

t-j-h left a comment

richsalz commented Apr 7, 2018

		@@ -1,26 +1,26 @@
		HOW TO CONTRIBUTE PATCHES TO OpenSSL
		HOW TO CONTRIBUTE TO PATCHES OpenSSL


		1. Anything other than trivial contributions will require a contributor
		To make it easier to review and accept our pull request, please follow these

Minor updates #5889

Minor updates #5889

Conversation

richsalz commented Apr 5, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

levitte commented Apr 5, 2018

t-j-h commented Apr 5, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

levitte left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

richsalz commented Apr 6, 2018

mspncp left a comment

Choose a reason for hiding this comment

richsalz commented Apr 6, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mspncp commented Apr 6, 2018 • edited

richsalz commented Apr 7, 2018

t-j-h left a comment

Choose a reason for hiding this comment

richsalz commented Apr 7, 2018

mspncp commented Apr 6, 2018 •

edited