Skip to content

Update and extend NOTES.WIN, adding 'Quick start' subsection#12098

Closed
DDvO wants to merge 5 commits intoopenssl:masterfrom
siemens:improve_NOTES.WIN
Closed

Update and extend NOTES.WIN, adding 'Quick start' subsection#12098
DDvO wants to merge 5 commits intoopenssl:masterfrom
siemens:improve_NOTES.WIN

Conversation

@DDvO
Copy link
Contributor

@DDvO DDvO commented Jun 9, 2020

This PR has been motivated by a hick-up with recent AcitvePerl-5.28, see #12068. It

@DDvO DDvO force-pushed the improve_NOTES.WIN branch from a1666e5 to 4986fa3 Compare June 9, 2020 12:22
@DDvO
Copy link
Contributor Author

DDvO commented Jun 9, 2020

BTW, any chance to rename the NOTES.WIN file to, e.g., NOTES-WIN.txt?
Since the WIN-dows file explorer does not know how to open a WIN File 😉

@DDvO DDvO force-pushed the improve_NOTES.WIN branch from 4986fa3 to 42625db Compare June 9, 2020 12:49
@DDvO
Copy link
Contributor Author

DDvO commented Jun 9, 2020

I've just found that NOTES.PERL also needed an update regarding ActivePerl; done.

@levitte levitte added approval: done This pull request has the required number of approvals branch: master Applies to master branch branch: 1.1.1 Applies to OpenSSL_1_1_1-stable branch (EOL) labels Jun 9, 2020
@mattcaswell
Copy link
Member

I wonder if we could re-write the opening section in NOTES.WIN. The "wall of text" is off putting IMO.

@levitte
Copy link
Member

levitte commented Jun 9, 2020

I wonder if we could re-write the opening section in NOTES.WIN. The "wall of text" is off putting IMO.

I would take it away.

@DDvO DDvO force-pushed the improve_NOTES.WIN branch from 42625db to bc942f2 Compare June 9, 2020 13:47
@DDvO
Copy link
Contributor Author

DDvO commented Jun 9, 2020

I wonder if we could re-write the opening section in NOTES.WIN. The "wall of text" is off putting IMO.

I've tentatively moved it down to where it belongs: the MinGW section.

@richsalz
Copy link
Contributor

richsalz commented Jun 9, 2020

Renaming the file, as pointed out in #12098 (comment), seems like a real aide to newcomers.

@levitte
Copy link
Member

levitte commented Jun 9, 2020

What about the other notes?
Also, they are largely markdown formatted, is .txt an appropriate extension? In that case, should we look at the extension for README and INSTALL one more time?

@DDvO
Copy link
Contributor Author

DDvO commented Jun 9, 2020

What about the other notes?
Also, they are largely markdown formatted, is .txt an appropriate extension? In that case, should we look at the extension for README and INSTALL one more time?

I'd take the .md extension for all MarkDown formatted files,
and .txt for everything else, in particular the NOTES.* files.
Or even better, convert the latter ones to MarkDown.

@richsalz
Copy link
Contributor

richsalz commented Jun 9, 2020

Strong agreement with @DDvO's comment #12098 (comment) Convert to md and use the .md extension else use the .txt extension and fix the names of those files.

@levitte
Copy link
Member

levitte commented Jun 9, 2020

Does Windows understand the .md extension out of the box?
Anyway, the NOTES files are already largely md (you could call it md-ish)

@DDvO
Copy link
Contributor Author

DDvO commented Jun 9, 2020

Does Windows understand the .md extension out of the box?

Sigh, no. So I suggest using the .txt extension for the NOTES.WIN file (only).

Anyway, the NOTES files are already largely md (you could call it md-ish)

Yes, so it's not a big effort to check if they look fine after renaming them to .md
and potentially adapt their contents as far as needed. I can do that when agreed.

@DDvO
Copy link
Contributor Author

DDvO commented Jun 9, 2020

Strong agreement with @DDvO's comment #12098 (comment) Convert to md and use the .md extension else use the .txt extension and fix the names of those files.

Ah, so I think I can take this as agreement.
Will do by tomorrow (I have to leave now).

@levitte
Copy link
Member

levitte commented Jun 9, 2020

My i suggest that the general mdification be made as another PR?

@DDvO
Copy link
Contributor Author

DDvO commented Jun 10, 2020

My i suggest that the general modification be made as another PR?

No problem - I can even do this in a parallel PR since that one won't touch the contents of NOTES.WIN.

@levitte
Copy link
Member

levitte commented Jun 10, 2020

No problem - I can even do this in a parallel PR since that one won't touch the contents of NOTES.WIN.

Thank you. I was afraid that this PR would grow indefinitely as new ideas are tossed at you, and I'm sure you've noticed, big PRs have a disproportionally longer processing time (for understandable reasons, human fallacy 'n all that)

@DDvO DDvO force-pushed the improve_NOTES.WIN branch from 7c8759f to fe43c85 Compare June 10, 2020 08:26
Copy link
Member

@mattcaswell mattcaswell left a comment

Choose a reason for hiding this comment

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

I did another pass on this. A few more comments. Nearly there I think.

Copy link
Member

@mattcaswell mattcaswell left a comment

Choose a reason for hiding this comment

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

This is sooo much better than what we had before!

@openssl-machine
Copy link
Collaborator

24 hours has passed since 'approval: done' was set, but as this PR has been updated in that time the label 'approval: ready to merge' is not being automatically set. Please review the updates and set the label manually.

openssl-machine pushed a commit that referenced this pull request Jun 13, 2020
Reviewed-by: Richard Levitte <[email protected]>
Reviewed-by: Matt Caswell <[email protected]>
(Merged from #12098)
@DDvO
Copy link
Contributor Author

DDvO commented Jun 13, 2020

Merged - thanks @levitte, @richsalz, and @mattcaswell

@DDvO DDvO closed this Jun 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approval: done This pull request has the required number of approvals branch: master Applies to master branch branch: 1.1.1 Applies to OpenSSL_1_1_1-stable branch (EOL)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants