Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom

Remove deprecations to support ember 4.x #90

Merged
merged 8 commits into from
Sep 21, 2022
Merged

Remove deprecations to support ember 4.x #90

merged 8 commits into from
Sep 21, 2022

Conversation

fsmanuel
Copy link
Collaborator

@fsmanuel fsmanuel commented Sep 19, 2022 •
edited
Loading

Ref #85

@fsmanuel fsmanuel self-assigned this Sep 19, 2022
@fsmanuel fsmanuel mentioned this pull request Sep 20, 2022
Merged
tehhowch
tehhowch reviewed Sep 20, 2022
View reviewed changes
@fsmanuel
Run yarn lint:js:fix with plugin:prettier/recommended
a1565d9
@fsmanuel
Run yarn lint:js:fix
4c9ed2c
@fsmanuel
Remove ember-open-browser
461ea5f
@fsmanuel
Remove ember-getowner-polyfill
f687726 
> ember-getowner-polyfill is not required for Ember 2.3.0 and later, please remove from your `package.json`.
@fsmanuel fsmanuel force-pushed the linting branch 2 times, most recently from 6bee291 to d5cf865 Compare September 21, 2022 07:58
@fsmanuel
Copy link
Collaborator Author

@ijlee2 @tehhowch everything except embroider-optimized is now green.

@fsmanuel fsmanuel requested a review from ijlee2 September 21, 2022 08:23
@fsmanuel fsmanuel changed the title Enable full linting and remove deprecations Remove deprecations to support ember 4.x Sep 21, 2022
ijlee2
ijlee2 reviewed Sep 21, 2022
View reviewed changes
tests/dummy/app/controllers/application.js Outdated
@action
update(attribute, event) {
let value = event.target.value;
return this.model.set(attribute, value);
Copy link
Contributor

@ijlee2 ijlee2 Sep 21, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

It feels a bit iffy to modify model (I like to think of what the model hook returns as read-only), but I understand that the PR is focused on updating the current syntax.

I might suggest that, in the future, we look at defining the validations and and the form "object" directly in the controller (currently these are defined in tests/dummy/app/models/dummy.js).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Happy to do a review if you open a PR =)

ijlee2
ijlee2 approved these changes Sep 21, 2022
View reviewed changes
Copy link
Contributor

@ijlee2 ijlee2 left a comment

Choose a reason for hiding this comment

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

✨

tehhowch
tehhowch reviewed Sep 21, 2022
View reviewed changes
@fsmanuel fsmanuel mentioned this pull request Sep 21, 2022
Open
@fsmanuel fsmanuel merged commit b18b1a9 into main Sep 21, 2022
@delete-merged-branch delete-merged-branch bot deleted the linting branch September 21, 2022 12:35
@ijlee2 ijlee2 mentioned this pull request May 30, 2023
Closed
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@tehhowch tehhowch tehhowch left review comments

@ijlee2 ijlee2 ijlee2 approved these changes

Assignees

@fsmanuel fsmanuel

Labels
enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@fsmanuel @ijlee2 @tehhowch