Skip to content

Conversation

jloughry
Copy link
Contributor

Minor spelling fix: the message AdvertizeLocal: advertizing address 168.103.195.250:8333 in the debug log was misspelt.

@laanwj
Copy link
Member

laanwj commented Feb 12, 2016

Isn't that just US versus UK spelling?

@jloughry
Copy link
Contributor Author

I've lived in US and UK and both countries spell it 'advertise'. It's just one of those weird edge cases in English.

References:

  1. http://grammarist.com/spelling/advertise-vs-advertize/
  2. The Oxford Style Guide ('Fowler') prefers '-ise' in this particular case.

@paveljanik
Copy link
Contributor

Can you please fix them all in the source when you are at it?

@jloughry
Copy link
Contributor Author

Fixed everywhere in src and doc.

@paveljanik
Copy link
Contributor

Please check by git grep advertiz:

src/main.cpp:                    LogPrintf("ProcessMessages: advertizing address %s\n", addr.ToString());
src/main.cpp:                    LogPrintf("ProcessMessages: advertizing address %s\n", addr.ToString());
src/net.cpp:            LogPrintf("AdvertiseLocal: advertizing address %s\n", addrLocal.ToString());
src/torcontrol.cpp:        LogPrintf("tor: Got service ID %s, advertizing service %s\n", service_id, service.ToString());
src/torcontrol.cpp:    // Stop advertizing service when disconnected

@jloughry
Copy link
Contributor Author

You're right. The remaining ones have been caught and fixed now. You taught me a new tool---thanks!

@paveljanik
Copy link
Contributor

@jloughry great! Now can you please squash two commits into one? The same tool, different command line arguments :-)

@jloughry
Copy link
Contributor Author

I squashed all my changes into the 37767fd commit, but the pull request has 4 commits in it now: the two original ones that were squashed, plus something new: cb5844d, that appears to be an empty merge. I can't figure out how to clean up the pull request so it contains only one commit:

  • 37767fd (This one contains all the changes: keep this one)
  • 2ef3edf (first commit, squashed into 37767fd)
  • 723551b (second commit, squashed into 37767fd)
  • cb5944d (I don't know what this is; it seems to be an empty merge)

How do I edit the pull request to remove the unneeded commits? Or is this normal behaviour when squashing commits? Thanks for the guidance; I want to do it right.

@achow101
Copy link
Member

@jloughry reset git so that HEAD is the commit you want it to be (37767fd) and then force push it to github

@jloughry jloughry force-pushed the advertize-advertise branch from cb5944d to 37767fd Compare February 13, 2016 03:56
@jloughry
Copy link
Contributor Author

Done---there's only one commit in the pull request now. Thank you for showing me how to do it.

@paveljanik
Copy link
Contributor

ACK jloughry@37767fd

@laanwj
Copy link
Member

laanwj commented Feb 13, 2016

@jloughry Thanks for the explanation, can't believe this was missed all this time but here we are.

Concept ACK.

@maflcko
Copy link
Member

maflcko commented Feb 13, 2016

You can use *-ize as well: https://en.wiktionary.org/wiki/advertize. Still, I am ok with 37767fd if it get's backported, so we don't create unnecessary backport conflicts.

@jloughry
Copy link
Contributor Author

A question of procedure: should backport changes be added to this pull request, or made one or more new pull requests?

@sipa
Copy link
Member

sipa commented Feb 15, 2016

You should typically only create pull requests against master; we'll backport changes to older releases as needed. In case a patch specifically applies to a backport only (like a change to 0.12 release notes) or is highly nontrivial to apply to older code (due to refactoring that may have taken place), you can create pull requests against specific branches.

@jloughry
Copy link
Contributor Author

Thanks for explaining it.

@laanwj
Copy link
Member

laanwj commented Feb 15, 2016

So what is it, can you use advertize or not?
If it is remotely valid, I'd prefer to keep it as it (as @MarcoFalke says this will create rebasing conflicts), if it looks stupid to native English speakers it should be changed.

@wtogami
Copy link
Contributor

wtogami commented Feb 15, 2016

It looks stupid to me, a native English speaker. I might have used it a few times only as it seemed to be an awkward Bitcoin term and I was talking about the peer manager at the time.

@laanwj laanwj merged commit 37767fd into bitcoin:master Feb 16, 2016
laanwj added a commit that referenced this pull request Feb 16, 2016
37767fd fix spelling of advertise in src and doc (jloughry)
maflcko pushed a commit to maflcko/bitcoin-core that referenced this pull request Apr 27, 2016
@maflcko
Copy link
Member

maflcko commented Jun 9, 2016

Backported as part of #7938. Removing label 'Needs backport'.

@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants