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

Improve tools::OpenSSL-Query/README.md #58

Closed
wants to merge 3 commits into from

Conversation

DDvO
Copy link
Contributor

@DDvO DDvO commented Feb 11, 2020

adds missing dependency info to the Testing section
allows simple copy&paste for the series of shell commands

OpenSSL-Query/README.md Outdated Show resolved Hide resolved
@levitte levitte added the ready label Feb 11, 2020
@DDvO
Copy link
Contributor Author

DDvO commented Feb 13, 2020

Since meanwhile more than a day has passed that this PR has receive two OTC approvals and has been marked 'ready' I've been trying to push this to master at openssl-git@git.openssl.org:tools.git

Yet I get write access denied:

Pushing to openssl-git@git.openssl.org:tools.git
FATAL: W any tools dev@ddvo.net DENIED by fallthru
(or you mis-spelled the reponame)
fatal: Could not read from remote repository.

Is maybe also here an inconsistency with my alternative email address,
or are only OTC members allowed to push to this repo?

@romen
Copy link
Member

romen commented Feb 13, 2020

This repo still belongs to OMC at the moment IIRC

@levitte
Copy link
Member

levitte commented Feb 13, 2020

I've raised the ownership question with the rest of the OMC. Meanwhile, I'm merging this now, for the sake of expedience.

@levitte
Copy link
Member

levitte commented Feb 13, 2020

Merged

d250d42 allow simple copy&paste for the series of shell commands in OpenSSL-Query/README.md
c217de2 add missing dependency info to the Testing section of OpenSSL-Query/README.md

@levitte levitte closed this Feb 13, 2020
@levitte
Copy link
Member

levitte commented Feb 13, 2020

Argh... I forgot to edit the messages to correct the merge URLs :-/

@mattcaswell
Copy link
Member

I forgot to edit the messages

You do know that addrev can write the correct URLs in the first place right? i.e. via the "--tools" option

@DDvO
Copy link
Contributor Author

DDvO commented Feb 13, 2020

Argh... I forgot to edit the messages to correct the merge URLs :-/

Thanks @levitte for taking over the merge.
I don't see anything wrong with it :)

@levitte
Copy link
Member

levitte commented Feb 13, 2020

You do know that addrev can write the correct URLs in the first place right? i.e. via the "--tools" option

I actually hadn't discovered that... and now I see --web too... it's really not the best of interfaces, I would rather have something that looks at the remote or something like that.
I've been thinking that addrev needs a rewrite anyway, it makes too much duplicate work (every separate commit that's to be rewritten generates that exact same calls to the people DB, that's a huge waste of time).... it won't happen immediately, though.

@levitte
Copy link
Member

levitte commented Feb 13, 2020

I don't see anything wrong with it :)

    (Merged from https://github.com/openssl/openssl/pull/58)

Have a hard look. I totally expect an "Ah-ha!!!" from you 😉

@DDvO
Copy link
Contributor Author

DDvO commented Feb 13, 2020

Ah, yes, @levitte, I must have had tomatoes on my eyes 😉
(which is the literal translation of a German saying with obvious meaning)

@DDvO
Copy link
Contributor Author

DDvO commented Feb 13, 2020

BTW, with the enhanced version of ghmerge I'm currently setting up in #59,
the merge could have been done very conveniently in a little error-prone manner like this:

ghmerge --tools --nomerge 58 paulidale levitte

@levitte
Copy link
Member

levitte commented Feb 13, 2020

... ok. I promise I'll have a closer look, but do expect scepticism until I warm up to it, 'k?

@DDvO
Copy link
Contributor Author

DDvO commented Feb 14, 2020

Meanwhile, the above example use of ghmerge has simplified to

ghmerge --tools 58 paulidale levitte

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants