Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign up[guide] Non-code issues: keep them out of your code files. #1643
Conversation
mk-pmb
force-pushed the
mk-pmb:explain-no-transport
branch
3 times, most recently
from
c6d3199
to
0bc58d1
|
rebased on current master. |
|
This section seems like it's full of "don't"s, without any "do's". Can you be more specific about what kinds of things this advice would avoid, and what better alternatives would look like? |
|
For clearer dos and don'ts about the BOM, we could use code blocks that contain just explanatory comments. Would at least keep the visual style consistent: /* <- bad: Invisible BOM at start of code file, meant to
fix your editor's charset.
good: Set the charset in your editor config.
If you need per-project config and your editor
doesn't support it, get a better one. *//* <- bad: Invisible BOM at start of code file, meant to
fix some browser's charset guessing.
good: Use your webserver or some part of your toolchain
to add the charset info. */ |
|
Draft of example for omitting potential UMD wrappers: // bad: Bloat project code with AMD/UMD export.
(function unifiedExport(e) {
var d = ((typeof define === 'function') && define),
m = ((typeof module === 'object') && module);
if (d && d.amd) { d(function () { return e; }); }
if (m && m.exports) { m.exports = e; }
}(api));
// good:
// Just omit it. Use your toolchain to add compatibility
// mechanisms only in the dist version.Edit: Not a good example though, since an export statement would prevent old browsers that need AMD/UMD from parsing the script at all. Overlaps with rule 10.1. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
mk-pmb commentedNov 22, 2017
Summary of #1640. I extrapolated the reasoning to other transport layer packaging, hope it's the right conclusion.