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

enforce rules on code blocks style in docs #11178

Merged
merged 3 commits into from Nov 21, 2017

Conversation

Projects
None yet
3 participants
@vanessayuenn
Copy link
Contributor

vanessayuenn commented Nov 20, 2017

This PR adds two rules to enforce proper code block styles (i.e. fenced with ```, and have a language specified per block), and fixes the resulting lint errors in our current docs. This will resolve electron/electronjs.org#868 where improper code blocks are not being styled on the website.

@vanessayuenn vanessayuenn requested review from as code owners Nov 20, 2017

@zeke
Copy link
Member

zeke left a comment

Awesome! I like that you addressed this problem at the source, rather than in i18n or electronjs.org 👍

@@ -18,7 +18,9 @@ See also [V8 Development](v8-development.md)
It is possible to debug Chromium with Electron by passing
`--build_debug_libcc` to the bootstrap script:

$ ./script/bootstrap.py -d --build_debug_libcc
```bash

This comment has been minimized.

@zeke

zeke Nov 20, 2017

Member

Any particular reason some of these are bash and some are sh? We should probably just pick one. My vote would be sh because it's more generic but still always accurate.

This comment has been minimized.

@vanessayuenn

vanessayuenn Nov 21, 2017

Author Contributor

We have both bash and sh scattered all over the docs; I think if we were to fix them, we should open up another PR for it. Currently, the designated language for each code block isn't actually taken into account how it's rendered.

This comment has been minimized.

@ckerr

ckerr Nov 21, 2017

Member

+1 on preferring sh over bash in documentation AFAIK we're not using any bash-only extensions and any variant, e.g. dash, would also work

+1 on deferring the s/bash/sh/ changes to a followup patch

@vanessayuenn

This comment has been minimized.

Copy link
Contributor Author

vanessayuenn commented Nov 20, 2017

Getting a ci error only on jenkins machine 🤔

+ script/test.py --ci --rebuild_native_modules

> dugite@1.49.0 postinstall /Users/electron/workspace/tron_fenced-code-block-lang-M7U2EJL7LXFSOF7ZETWS2SQEF3E4653OZZHHFFE3DXCBGPCEJIMQ/node_modules/dugite
> node ./script/download-git.js


> husky@0.14.3 install /Users/electron/workspace/tron_fenced-code-block-lang-M7U2EJL7LXFSOF7ZETWS2SQEF3E4653OZZHHFFE3DXCBGPCEJIMQ/node_modules/husky
> node ./bin/install.js

husky
CI detected, skipping Git hooks installation

> fsevents@1.1.3 install /Users/electron/workspace/tron_fenced-code-block-lang-M7U2EJL7LXFSOF7ZETWS2SQEF3E4653OZZHHFFE3DXCBGPCEJIMQ/node_modules/fsevents
> node install

node-pre-gyp ERR! install error 
node-pre-gyp ERR! stack Error: Unknown target version: v1.8.2-beta.2
node-pre-gyp ERR! stack     at get_runtime_abi (/Users/electron/workspace/tron_fenced-code-block-lang-M7U2EJL7LXFSOF7ZETWS2SQEF3E4653OZZHHFFE3DXCBGPCEJIMQ/node_modules/fsevents/node_modules/node-pre-gyp/lib/util/versioning.js:97:27)
node-pre-gyp ERR! stack     at Object.module.exports.evaluate (/Users/electron/workspace/tron_fenced-code-block-lang-M7U2EJL7LXFSOF7ZETWS2SQEF3E4653OZZHHFFE3DXCBGPCEJIMQ/node_modules/fsevents/node_modules/node-pre-gyp/lib/util/versioning.js:293:19)
node-pre-gyp ERR! stack     at install (/Users/electron/workspace/tron_fenced-code-block-lang-M7U2EJL7LXFSOF7ZETWS2SQEF3E4653OZZHHFFE3DXCBGPCEJIMQ/node_modules/fsevents/node_modules/node-pre-gyp/lib/install.js:165:31)
node-pre-gyp ERR! stack     at Object.self.commands.(anonymous function) [as install] (/Users/electron/workspace/tron_fenced-code-block-lang-M7U2EJL7LXFSOF7ZETWS2SQEF3E4653OZZHHFFE3DXCBGPCEJIMQ/node_modules/fsevents/node_modules/node-pre-gyp/lib/node-pre-gyp.js:50:37)
node-pre-gyp ERR! stack     at run (/Users/electron/workspace/tron_fenced-code-block-lang-M7U2EJL7LXFSOF7ZETWS2SQEF3E4653OZZHHFFE3DXCBGPCEJIMQ/node_modules/fsevents/node_modules/node-pre-gyp/bin/node-pre-gyp:82:30)
node-pre-gyp ERR! stack     at Object.<anonymous> (/Users/electron/workspace/tron_fenced-code-block-lang-M7U2EJL7LXFSOF7ZETWS2SQEF3E4653OZZHHFFE3DXCBGPCEJIMQ/node_modules/fsevents/node_modules/node-pre-gyp/bin/node-pre-gyp:134:1)
node-pre-gyp ERR! stack     at Module._compile (module.js:570:32)
node-pre-gyp ERR! stack     at Object.Module._extensions..js (module.js:579:10)
node-pre-gyp ERR! stack     at Module.load (module.js:487:32)
node-pre-gyp ERR! stack     at tryModuleLoad (module.js:446:12)
node-pre-gyp ERR! System Darwin 16.7.0
node-pre-gyp ERR! command "/usr/local/bin/node" "/Users/electron/workspace/tron_fenced-code-block-lang-M7U2EJL7LXFSOF7ZETWS2SQEF3E4653OZZHHFFE3DXCBGPCEJIMQ/node_modules/fsevents/node_modules/node-pre-gyp/bin/node-pre-gyp" "install" "--fallback-to-build"
node-pre-gyp ERR! cwd /Users/electron/workspace/tron_fenced-code-block-lang-M7U2EJL7LXFSOF7ZETWS2SQEF3E4653OZZHHFFE3DXCBGPCEJIMQ/node_modules/fsevents
node-pre-gyp ERR! node -v v6.10.2
node-pre-gyp ERR! node-pre-gyp -v v0.6.39
node-pre-gyp ERR! not ok 
Unknown target version: v1.8.2-beta.2
@vanessayuenn

This comment has been minimized.

Copy link
Contributor Author

vanessayuenn commented Nov 21, 2017

🔍 🐞 So @MarshallOfSound and I found a bug in the util.py script where the working directory is left out without being passed through if execute is run without the verbose flag; so the rebuild_native_modules was trying to rebuild the root node modules instead of only the spec ones. Fixing that made the builds green 💚 yay!

@ckerr

ckerr approved these changes Nov 21, 2017

Copy link
Member

ckerr left a comment

Nice patch :)

@ckerr ckerr merged commit 70643a8 into master Nov 21, 2017

6 checks passed

ci/circleci: electron-linux-arm Your tests passed on CircleCI!
Details
ci/circleci: electron-linux-arm64 Your tests passed on CircleCI!
Details
ci/circleci: electron-linux-ia32 Your tests passed on CircleCI!
Details
ci/circleci: electron-linux-x64 Your tests passed on CircleCI!
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/jenkins/pr-head This commit looks good
Details

@ckerr ckerr deleted the fenced-code-block-lang branch Nov 21, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.