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.

Sign up for GitHub

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

Jump to bottom

fix some inconsistencies #1173

Merged
merged 2 commits into from
Nov 22, 2019
Merged

fix some inconsistencies #1173

merged 2 commits into from
Nov 22, 2019

Conversation

sly7-7
Copy link
Contributor

@sly7-7 sly7-7 commented Nov 13, 2019

Not sure about all things, so please correct me if I'm wrong.

@sly7-7 sly7-7 mentioned this pull request Nov 14, 2019
Closed
30 tasks
jenweber
jenweber reviewed Nov 15, 2019
View reviewed changes
Copy link
Contributor

@jenweber jenweber left a comment

Choose a reason for hiding this comment

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

Thanks for these! I appreciate the attention to detail. These kinds of inconsistencies are confusing for new devs.

We haven't really tried to enforce any consistency with ; or " but somehow we should try to lint for this in the future.

guides/release/routing/specifying-a-routes-model.md Outdated Show resolved Hide resolved
@jenweber
Copy link
Contributor

There is one change that needs to be made before we can merge, in the code suggestions above. If you would like maintainers to be able to fix typos and things in order to get your PR merged faster, click the checkbox "allow maintainers to make edits" when you open a PR. It's not required but it helps! I'm not able to apply this suggestion myself with the current settings.

@sly7-7
Copy link
Contributor Author

sly7-7 commented Nov 15, 2019

@jenweber Sorry for missing that. I wish this feature be enabled by default... Thank you for the review, this should be good now.

@sly7-7
Copy link
Contributor Author

sly7-7 commented Nov 15, 2019

We haven't really tried to enforce any consistency with ; or " but somehow we should try to lint for this in the future.

For that, I guess that's my vscode config, or the project editor config. VSCode has done the transform on save.

@locks locks self-assigned this Nov 15, 2019
@jenweber jenweber self-requested a review November 22, 2019 00:15
jenweber
jenweber approved these changes Nov 22, 2019
View reviewed changes
Copy link
Contributor

@jenweber jenweber left a comment

Choose a reason for hiding this comment

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

thanks for the fixes!

@jenweber jenweber merged commit 73a2452 into ember-learn:octane Nov 22, 2019
@sly7-7 sly7-7 deleted the fix-specifying-route-models branch November 22, 2019 08:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jenweber jenweber jenweber approved these changes

Assignees

@locks locks

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@sly7-7 @jenweber @locks