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.

Sign up for GitHub

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

Jump to bottom

doc: Fix whitespace errs in .md files, bitcoin.conf, and Info.plist.in #16812

Merged
merged 1 commit into from
Sep 17, 2019
Merged

doc: Fix whitespace errs in .md files, bitcoin.conf, and Info.plist.in #16812

merged 1 commit into from
Sep 17, 2019

Conversation

ch4ot1c
Copy link
Contributor

@ch4ot1c ch4ot1c commented Sep 5, 2019

Although there is an existing test/lint/lint-whitespace.sh linter, it only prevents new errors from being introduced. This commit removes all existing whitespace errors from Core markdown files (skips src/crypto/ctaes/, leveldb/, and doc/release-notes/), bitcoin.conf, and Info.plist.in.

Further formatting could be done on the markdown documents, but seeing as there several coexisting styles that break a few markdownlint rules, a first step would be to define and add a linter to Travis. For now, the small fix is made.

@GChuf
Copy link
Contributor

GChuf commented Sep 5, 2019 •
edited
Loading

Concept ACK

promag
promag reviewed Sep 6, 2019
View reviewed changes
doc/REST-interface.md Outdated Show resolved Hide resolved
@DrahtBot
Copy link
Contributor

DrahtBot commented Sep 6, 2019

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #16392 (WIP build: macOS toolchain update by fanquake)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

@fanquake
Copy link
Member

fanquake commented Sep 6, 2019

The script doesn't run on Travis:

Running script for: ac5b890fb1b05fc2d8df7e876fa94d045bf6fbc0
shopt -s globstar
sed -i 's/[ 	]*$//g' {build_msvc,ci,contrib,depends,test}/*.md doc/*.{md,txt} contrib/**/*.{md,txt} share/**/*.{conf,plist.in,md} test/**/*.md
test/lint/commit-script-check.sh: 1: eval: shopt: not found
sed: can't read {build_msvc,ci,contrib,depends,test}/*.md: No such file or directory
sed: can't read doc/*.{md,txt}: No such file or directory
sed: can't read contrib/**/*.{md,txt}: No such file or directory
sed: can't read share/**/*.{conf,plist.in,md}: No such file or directory

@ch4ot1c ch4ot1c force-pushed the docs/lint-markdown branch 5 times, most recently from df0ddd1 to 48cef93 Compare September 8, 2019 03:53
@ch4ot1c
Copy link
Contributor Author

ch4ot1c commented Sep 8, 2019 •
edited
Loading

For some reason, the script is skipping build-aux/ in Travis, which accounts for a single correction. I've tried it with and without both quotes and escaped dash. It runs correctly on all my linux environments - is there a bashism I'm missing?

Could remove the touch to the .m4 file, instead, but it would be nice to get this scripted-diff working. Inspired by @promag's 12dd101

Edit: Final script was:

-BEGIN VERIFY SCRIPT-
sed -i -r 's/[ \t]*$//g' \
$(git grep -IlE '[ \t]*$' -- \
build_msvc/*.{md,in,sln,vcxproj} \
"build-aux"/m4/*.m4 \
ci/*.md \
ci/**/*.md \
contrib/*.{"bash-completion",md,pro,supp} \
contrib/**/*.{cfg,in,md,plist,scm,txt,yml} \
depends/*.{md,mk} \
depends/**/*.{md,mk} \
doc/*.{in,md,txt} \
share/*.in \
share/**/*.{conf,in,md,txt} \
src/**/*.md \
test/*.md \
test/**/*.{in,md} \
':!src/crypto/ctaes' \
':!src/leveldb' \
':!src/secp256k1' \
':!src/univalue' \
)
-END VERIFY SCRIPT-

@promag
Copy link
Contributor

promag commented Sep 15, 2019

Concept ACK.

@fanquake
Copy link
Member

To be honest I'm not really concerned about the scripted diff here. A single commit that removes the last of the (impossible to re-introduce) whitespace should be fine.

@practicalswift
Copy link
Contributor

ACK 48cef93 -- diff looks correct

@maflcko
Copy link
Member

maflcko commented Sep 16, 2019

Can the scripted diff be removed, then?

@ch4ot1c
Copy link
Contributor Author

ch4ot1c commented Sep 17, 2019

Removed.

fanquake
fanquake approved these changes Sep 17, 2019
View reviewed changes
Copy link
Member

@fanquake fanquake left a comment

Choose a reason for hiding this comment

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

ACK 6aab764 - Thanks for following up. Hopefully we now never have to deal with whitespace again.

fanquake added a commit that referenced this pull request Sep 17, 2019
@fanquake
Merge #16812: doc: Fix whitespace errs in .md files, bitcoin.conf, an…
f18a906 
…d Info.plist.in

6aab764 doc: Fix whitespace errs in .md files, bitcoin.conf, Info.plist.in, and find_bdb48.m4 (Jon Layton)

Pull request description:

  Although there is an existing `test/lint/lint-whitespace.sh` linter, it only prevents new errors from being introduced. This commit removes all existing whitespace errors from Core markdown files (skips `src/crypto/ctaes/`, `leveldb/`, and `doc/release-notes/`), `bitcoin.conf`, and `Info.plist.in`.

  Further formatting could be done on the markdown documents, but seeing as there several coexisting styles that break a few `markdownlint` rules, a first step would be to define and add a linter to Travis. For now, the small fix is made.

ACKs for top commit:
  fanquake:
    ACK 6aab764 - Thanks for following up. Hopefully we now never have to deal with whitespace again.

Tree-SHA512: 810cc31ae4364b2dedf85783e67315d7b4e11589e4b32c599606e1b1ba8de0663bcae9ddb1bd8c9762a3636a2d65bdcd64ec22d2e90943f374a0c9574b77ca23
@fanquake fanquake merged commit 6aab764 into bitcoin:master Sep 17, 2019
Munkybooty pushed a commit to Munkybooty/dash that referenced this pull request Dec 7, 2021
@fanquake @Munkybooty
Merge bitcoin#16812: doc: Fix whitespace errs in .md files, bitcoin.c…
9f052da 
…onf, and Info.plist.in

6aab764 doc: Fix whitespace errs in .md files, bitcoin.conf, Info.plist.in, and find_bdb48.m4 (Jon Layton)

Pull request description:

  Although there is an existing `test/lint/lint-whitespace.sh` linter, it only prevents new errors from being introduced. This commit removes all existing whitespace errors from Core markdown files (skips `src/crypto/ctaes/`, `leveldb/`, and `doc/release-notes/`), `bitcoin.conf`, and `Info.plist.in`.

  Further formatting could be done on the markdown documents, but seeing as there several coexisting styles that break a few `markdownlint` rules, a first step would be to define and add a linter to Travis. For now, the small fix is made.

ACKs for top commit:
  fanquake:
    ACK 6aab764 - Thanks for following up. Hopefully we now never have to deal with whitespace again.

Tree-SHA512: 810cc31ae4364b2dedf85783e67315d7b4e11589e4b32c599606e1b1ba8de0663bcae9ddb1bd8c9762a3636a2d65bdcd64ec22d2e90943f374a0c9574b77ca23
Munkybooty pushed a commit to Munkybooty/dash that referenced this pull request Dec 15, 2021
@fanquake @Munkybooty
Merge bitcoin#16812: doc: Fix whitespace errs in .md files, bitcoin.c…
0723582 
…onf, and Info.plist.in

6aab764 doc: Fix whitespace errs in .md files, bitcoin.conf, Info.plist.in, and find_bdb48.m4 (Jon Layton)

Pull request description:

  Although there is an existing `test/lint/lint-whitespace.sh` linter, it only prevents new errors from being introduced. This commit removes all existing whitespace errors from Core markdown files (skips `src/crypto/ctaes/`, `leveldb/`, and `doc/release-notes/`), `bitcoin.conf`, and `Info.plist.in`.

  Further formatting could be done on the markdown documents, but seeing as there several coexisting styles that break a few `markdownlint` rules, a first step would be to define and add a linter to Travis. For now, the small fix is made.

ACKs for top commit:
  fanquake:
    ACK 6aab764 - Thanks for following up. Hopefully we now never have to deal with whitespace again.

Tree-SHA512: 810cc31ae4364b2dedf85783e67315d7b4e11589e4b32c599606e1b1ba8de0663bcae9ddb1bd8c9762a3636a2d65bdcd64ec22d2e90943f374a0c9574b77ca23
Munkybooty pushed a commit to Munkybooty/dash that referenced this pull request Dec 15, 2021
@fanquake @Munkybooty
Merge bitcoin#16812: doc: Fix whitespace errs in .md files, bitcoin.c…
939fd28 
…onf, and Info.plist.in

6aab764 doc: Fix whitespace errs in .md files, bitcoin.conf, Info.plist.in, and find_bdb48.m4 (Jon Layton)

Pull request description:

  Although there is an existing `test/lint/lint-whitespace.sh` linter, it only prevents new errors from being introduced. This commit removes all existing whitespace errors from Core markdown files (skips `src/crypto/ctaes/`, `leveldb/`, and `doc/release-notes/`), `bitcoin.conf`, and `Info.plist.in`.

  Further formatting could be done on the markdown documents, but seeing as there several coexisting styles that break a few `markdownlint` rules, a first step would be to define and add a linter to Travis. For now, the small fix is made.

ACKs for top commit:
  fanquake:
    ACK 6aab764 - Thanks for following up. Hopefully we now never have to deal with whitespace again.

Tree-SHA512: 810cc31ae4364b2dedf85783e67315d7b4e11589e4b32c599606e1b1ba8de0663bcae9ddb1bd8c9762a3636a2d65bdcd64ec22d2e90943f374a0c9574b77ca23
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Dec 16, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@promag promag promag left review comments

@fanquake fanquake fanquake approved these changes

Assignees
No one assigned
Labels
Docs Scripts and tools
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@ch4ot1c @GChuf @DrahtBot @fanquake @promag @practicalswift @maflcko @PiRK