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

feat: ember-cli-update #469

Merged
merged 20 commits into from
Apr 8, 2022
Merged

feat: ember-cli-update #469

merged 20 commits into from
Apr 8, 2022

Conversation

guidojw
Copy link
Member

@guidojw guidojw commented Feb 10, 2022

Summary

Tracks the ember-cli app blueprint to its current version in config/ember-cli-update.json so we can use ember-cli-update to update it. I then updated ember-cli to the last v3 version. Some order of lines in files was changed so that in the future when ember-cli-update is used, the amount of merge conflicts caused by the process should be minimised.

It looked like you can track any blueprint with ember-cli-update (like model, component, controller, route, ...), but it doesn't seem to actually support that (yet). For now, using this to update the base blueprint works pretty well.

Because of #501 and danlynn/ember-cli Docker image now using Node 16, I used yarn resolutions to force workerpool@^2.3.0 to ^2.3.4. When ember-crumbly is replaced, this can be removed.
Also @embroider/macros is forced to ^1.0.0 because of another error.

Other information

https://github.com/ember-cli/ember-cli-update
App blueprint: https://github.com/ember-cli/ember-new-output

@guidojw guidojw mentioned this pull request Feb 10, 2022
…blueprint

refactor(package): follow ember-new-output as close as possible

refactor(server): make proxies and server follow blueprints

chore(deps): specify moment-duration-format patch version

fix(ci): configure ember-cli-update correctly

fix(renovate): only include package and new value in PR

fix(renovate): change new value column to change

fix(renovate): don't group ember-cli updates

feat: save ember-cli update state

fix(ci): install after ember-cli-update with immutable false

fix(ci): use correct flag for mutable yarn install

fix(ci): fetch previous ref too for ember-cli-update

fix(ci): checkout code on head_ref

fix(ci): don't commit changes to .github

fix(ci): remove file directly instead of git rm

fix(ci): remove ember-cli-update step

refactor(renovate): group ember-cli deps again

fix(renovate): remove rangeStrategy for ember-cli package

fix(renovate): swap matchPackageNames

feat(ci): make patch ember-cli updates not draft PRs

fix(renovate): use correct dockerfile manager name

fix(renovate): change commitMessageTopic to exclude docker tags

feat(ci): readd ember-cli-update action

fix(ci): try fixing with syntax

refactor(ci): move ember-cli-update to own job

fix(ci): change column definition to max action regex

feat(ci): get token from app

fix(ci): add if to build step

fix(ci): change actor name

fix(ci): add status check to if

fix(ci): check if renovate is actor other way

fix(ci): move comment to hopefully fix autofix_command

fix(ci): run lint and test when build succeeds

chore(deps): downgrade danlynn/ember-cli

feat: track ember-data with ember-cli-update

feat: install ember-cli-update

fix: remove ember-cli from npm deps packageRules

fix(renovate): remove separateMinorPatch from ember-cli packageRules

fix(package): change ember-cli constraint

revert: changes from the prettier PR

feat: add missing files
@guidojw guidojw marked this pull request as ready for review March 16, 2022 13:32
'use strict';

const EmberApp = require('ember-cli/lib/broccoli/ember-app');

module.exports = function (defaults) {
const app = new EmberApp(defaults, {
let app = new EmberApp(defaults, {
Copy link
Contributor

Choose a reason for hiding this comment

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

why let?

Copy link
Member Author

Choose a reason for hiding this comment

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

This was automatically changed by the blueprint updates.

I see there's not a general consensus in the Ember community of which one to use (some big addon maintainers apparently prefer "liberal let" instead of "constantly const").
The default ESLint configuration that comes with the Ember app blueprint does not include any rule that enforces one or the other.

What should we do here?

  • We could allow both (Ember default app blueprint does that).
  • We could configure ESLint's prefer-const and keep reverting these automatic conversions by ember-cli-update.
  • We could install some ESLint plugin to enforce let.

Copy link
Contributor

Choose a reason for hiding this comment

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

eh, I think option 1 is best then, it doesn't really matter that much if it's this much of a discussion in the Ember community. I am just surprised by it, because I am used to getting bashed for not using cost instead of let whenever possible

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah yes I was one of those people 😂 apologies

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.

3 participants