Release: Use an in-repository dist README fixture #4614
Conversation
|
|
||
| npmTags = Release.npmTags; | ||
| const npmTags = Release.npmTags; |
There was a problem hiding this comment.
@timmywil We had a talk a while ago about the onevar rule in ESLint. You said you don't want to remove/change the rule so that code using the preset doesn't need to adapt. However:
- The rule is not actually enabled in the jQuery preset. AFAIK it was removed because Jörn didn't want it in jQuery UI or QUnit due to Git diff issues etc.
- While we adhere to this way of declaring variables in jQuery code, I'd argue it's actually harmful when used for
const/let. This is because forvars we were emulating how the JS engine interprets those declarations; in case ofconst/letthe declarations are not hoisted and it's common to put them in the middle of the function where they are needed. - Other ES6 code in our repo already uses separate declarations per variable so we have some history here. That also means we don't have history of requiring this rule for
const/letso not applying it shouldn't cause issues with code refactorings.
There was a problem hiding this comment.
The onevar rule can technically differentiate between var, const, and let. I'd be immediately in favor of specifying that we only require onevar for var. As for var, I'm not sure it's worth the large diffs and blame overrides.
There was a problem hiding this comment.
@timmywil Makes sense, I didn't know it can be configured just for var. We can add it to our own ESLint config since it was rejected as a general rule for the jQuery preset.
| var $ = require( "jquery" ); | ||
| ``` | ||
|
|
||
| #### AMD (Asynchronous Module Definition) |
There was a problem hiding this comment.
I'll submit a separate PR adding info about our new ES modules. Since we only want it on the 4.x branch, this PR will be useful here. :)
Use a dist README fixture kept in the jQuery repository instead of modifying an existing one. This makes the jQuery repository the single source of truth when it comes to jQuery releases and it makes it easier to make changes to README without worrying how it will affect older jQuery lines.
mgol
force-pushed
the
jquery-dist-fixtures
branch
from
cce7735 to
14b93d0
Compare
Use a dist README fixture kept in the jQuery repository instead of modifying an existing one. This makes the jQuery repository the single source of truth when it comes to jQuery releases and it makes it easier to make changes to README without worrying how it will affect older jQuery lines. The commit also ES6ifies build/release.js & build/release/dist.js Closes gh-4614 (cherry picked from commit 358b769)
Summary
Use a dist README fixture kept in the jQuery repository instead of modifying
an existing one. This makes the jQuery repository the single source of truth
when it comes to jQuery releases and it makes it easier to make changes to
README without worrying how it will affect older jQuery lines.
jquery/jquery-release#103 is a pre-requisite to make this PR work.
Checklist
New tests have been added to show the fix or feature worksIf needed, a docs issue/PR was created at https://github.com/jquery/api.jquery.com