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

Removes deprecated Private INVOKE API #19639

Merged
merged 2 commits into from
Jul 18, 2021
Merged

Removes deprecated Private INVOKE API #19639

merged 2 commits into from
Jul 18, 2021

Conversation

nlfurniss
Copy link
Contributor

@nlfurniss nlfurniss commented Jul 18, 2021

Removes deprecated Private INVOKE API, actions.custom-invoke-invokable

@chancancode
Copy link
Member

Thanks! The content of the PR doesn't seem to match the title, is that right? (Also, for future, do you mind rebasing instead of merging master into your branch to keep the history more linear?)

@nlfurniss
Copy link
Contributor Author

Thanks! The content of the PR doesn't seem to match the title, is that right? (Also, for future, do you mind rebasing instead of merging master into your branch to keep the history more linear?)

Yeah something is off, I’ll take a look tomorrow. Normally I would rebase, but GH GUI only allows you to fetch and merge in fork, not rebase 😕 (and I’m on my phone since my laptop is out of juice).

You can rebase and merge from the GUI though right?

@nlfurniss nlfurniss changed the title Remove application controller router properties Removes deprecated Private INVOKE API Jul 18, 2021
@nlfurniss
Copy link
Contributor Author

@chancancode okay figured out the discrepancy and fixed the title and desc

@mixonic mixonic merged commit d823959 into emberjs:master Jul 18, 2021
@mixonic
Copy link
Member

mixonic commented Jul 18, 2021

I merged this and a number of other PRs what contained a merge commit from master into the PR's branch, but for the future @nlfurniss FWIW I strongly agree with @chancancode that a clean history is strongly preferred. It really helps us deal with rollbacks or understanding a change to have a fairly linear history on master with semantic commits. Thanks for taking it into consideration in the future.

@mixonic mixonic mentioned this pull request Jul 18, 2021
58 tasks
@nlfurniss nlfurniss deleted the remove-application-controller-router-properties branch July 18, 2021 17:40
@nlfurniss
Copy link
Contributor Author

I merged this and a number of other PRs what contained a merge commit from master into the PR's branch, but for the future @nlfurniss FWIW I strongly agree with @chancancode that a clean history is strongly preferred. It really helps us deal with rollbacks or understanding a change to have a fairly linear history on master with semantic commits. Thanks for taking it into consideration in the future.

@mixonic for sure. It’s sadly not possible with the GH UI, and I was hoping the ember repo allowed “rebase and merge” from PRs to fix the issue. Going forward the ones I haven’t reopened I’ll rebase.

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

Successfully merging this pull request may close these issues.

3 participants