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

[FEATURE] Lint fix after blueprint generation #9469

Merged
merged 8 commits into from
Mar 8, 2021
Merged

[FEATURE] Lint fix after blueprint generation #9469

merged 8 commits into from
Mar 8, 2021

Conversation

rpemberton
Copy link
Contributor

@rpemberton rpemberton commented Mar 2, 2021

Fixes #9429

This adds auto lint fixing for files generated by ember generate and ember init.

This is my first PR on ember-cli so looking forward to any feedback and happy to make improvements to it.

@rpemberton rpemberton changed the title [feature] Lint fix after blueprint generation #9429 [FEATURE] Lint fix after blueprint generation #9429 Mar 2, 2021
@rpemberton rpemberton changed the title [FEATURE] Lint fix after blueprint generation #9429 [FEATURE] Lint fix after blueprint generation Mar 2, 2021
Copy link
Member

@rwjblue rwjblue left a comment

Choose a reason for hiding this comment

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

Overall this is great, thanks for working on it!!

I've left a few small inline comments to work on, but please ping me when you are ready for another review.

lib/commands/generate.js Outdated Show resolved Hide resolved
lib/tasks/generate-from-blueprint.js Outdated Show resolved Hide resolved
lib/tasks/install-blueprint.js Outdated Show resolved Hide resolved
lib/utilities/lint-fix.js Outdated Show resolved Hide resolved
async function run() {
let lintFixScriptName = `lint:fix`;
let cwd = process.cwd();
let npmRunResult = await execa('npm', ['run'], { cwd });
Copy link
Member

Choose a reason for hiding this comment

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

We should probably use either npm or yarn based on the normal "is yarn.lock present" trigger that we normally use

lib/utilities/lint-fix.js Outdated Show resolved Hide resolved
lib/utilities/lint-fix.js Outdated Show resolved Hide resolved
lib/utilities/lint-fix.js Outdated Show resolved Hide resolved
Copy link
Member

@rwjblue rwjblue left a comment

Choose a reason for hiding this comment

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

Looks really good, thanks for keeping at it!

I'm not 100% sure why CI is failing, but the actual code changes here look really good. I think you might need to rebase (we recently merged a few other PRs).

lib/tasks/generate-from-blueprint.js Outdated Show resolved Hide resolved
lib/tasks/install-blueprint.js Outdated Show resolved Hide resolved
Copy link
Member

@rwjblue rwjblue left a comment

Choose a reason for hiding this comment

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

Just one clarification, but I think this is good to go.

lib/utilities/lint-fix.js Outdated Show resolved Hide resolved
@rwjblue
Copy link
Member

rwjblue commented Mar 8, 2021

I restarted CI (macOS had a test timeout).

@rpemberton
Copy link
Contributor Author

rpemberton commented Mar 8, 2021

Thanks, yeh this test appears to be a little flaky:
macOS: Acceptance: ember new ember new npm blueprint with old version

@rwjblue rwjblue merged commit fc79145 into ember-cli:master Mar 8, 2021
@rwjblue
Copy link
Member

rwjblue commented Mar 8, 2021

Thank you!

@bertdeblock
Copy link

The reason for this is mentioned in the linked issue as well as in the corresponding RFC itself. Note that you can also disable this functionality globally via your .ember-cli config file.

@Z-Zhao
Copy link

Z-Zhao commented Jan 18, 2024

The reason for this is mentioned in the linked issue as well as in the corresponding RFC itself. Note that you can also disable this functionality globally via your .ember-cli config file.

Can anyone suggest how could I disable this lint:fix globally via the .ember-cli config file. As mentioned above?

@charlesfries
Copy link
Contributor

@Z-Zhao

// .ember-cli

{
  "lintFix": false
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Automatically run lint:fix after ember init and ember generate
6 participants