Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

fix spelling of advertise (shows up in the debug log) #7526

Merged
merged 1 commit into from Feb 16, 2016

Conversation

Projects
None yet
7 participants
Contributor

jloughry commented Feb 12, 2016

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

Owner

laanwj commented Feb 12, 2016

Isn't that just US versus UK spelling?

Contributor

jloughry commented Feb 12, 2016

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.
Contributor

paveljanik commented Feb 12, 2016

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

Contributor

jloughry commented Feb 12, 2016

Fixed everywhere in src and doc.

Contributor

paveljanik commented Feb 12, 2016

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
Contributor

jloughry commented Feb 12, 2016

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

Contributor

paveljanik commented Feb 12, 2016

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

Contributor

jloughry commented Feb 12, 2016

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.

Member

achow101 commented Feb 12, 2016

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

Contributor

jloughry commented Feb 13, 2016

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

Owner

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.

Member

MarcoFalke 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.

Contributor

jloughry commented Feb 14, 2016

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

Owner

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.

Contributor

jloughry commented Feb 15, 2016

Thanks for explaining it.

Owner

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.

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

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

laanwj added a commit that referenced this pull request Feb 16, 2016

Merge #7526: fix spelling of advertise (shows up in the debug log)
37767fd fix spelling of advertise in src and doc (jloughry)

MarcoFalke added a commit to MarcoFalke/bitcoin that referenced this pull request Apr 27, 2016

Member

MarcoFalke commented Jun 9, 2016

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

zander added a commit to zander/bitcoinclassic that referenced this pull request Jun 16, 2016

nomnombtc added a commit to nomnombtc/bitcoin that referenced this pull request Nov 12, 2016

nomnombtc added a commit to nomnombtc/bitcoin that referenced this pull request Nov 12, 2016

nomnombtc added a commit to nomnombtc/bitcoin that referenced this pull request Nov 13, 2016

sickpig added a commit to sickpig/BitcoinUnlimited that referenced this pull request Nov 14, 2016

fix spelling of advertise in src and doc
Github-Pull: #7526
Rebased-From: 37767fd
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment