-
Notifications
You must be signed in to change notification settings - Fork 26.5k
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
[guide] Non-code issues: keep them out of your code files. #1643
base: master
Are you sure you want to change the base?
Conversation
c6d3199
to
0bc58d1
Compare
0bc58d1
to
be07e51
Compare
rebased on current master. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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?
be07e51
to
4fd6ab3
Compare
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. |
4fd6ab3
to
6dbe1ed
Compare
Summary of #1640. I extrapolated the reasoning to other transport layer packaging, hope it's the right conclusion.