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

Clarify specification #12

Merged
merged 6 commits into from
Nov 12, 2019
Merged

Clarify specification #12

merged 6 commits into from
Nov 12, 2019

Conversation

jednano
Copy link
Member

@jednano jednano commented Nov 9, 2019

I had a great time hanging out with @florianb in Berlin yesternight and we hashed out which parts of the EditorConfig specification needed clarification. This PR is the result of these conversations, combined with a couple of open issues.

Fixes #8
Fixes #2

index.rst Outdated Show resolved Hide resolved
index.rst Show resolved Hide resolved
index.rst Outdated Show resolved Hide resolved
index.rst Outdated Show resolved Hide resolved
index.rst Show resolved Hide resolved
@jednano jednano self-assigned this Nov 9, 2019
@jednano jednano requested a review from ffes November 9, 2019 23:23
index.rst Outdated Show resolved Hide resolved
index.rst Outdated Show resolved Hide resolved
index.rst Outdated Show resolved Hide resolved
index.rst Show resolved Hide resolved
index.rst Show resolved Hide resolved
@jednano
Copy link
Member Author

jednano commented Nov 11, 2019

@xuhdev @florianb please take another look. I believe I've addressed all of the issues presented. Thanks for the review!

Copy link
Member

@florianb florianb left a comment

Choose a reason for hiding this comment

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

LGTM - Thank you for your work @jedmao!

Copy link
Member

@xuhdev xuhdev left a comment

Choose a reason for hiding this comment

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

Content wise I think the PR is good. I suggested a few wording changes -- Hope this is helpful!

index.rst Outdated Show resolved Hide resolved
index.rst Outdated Show resolved Hide resolved
index.rst Outdated Show resolved Hide resolved
index.rst Outdated Show resolved Hide resolved
index.rst Outdated Show resolved Hide resolved
index.rst Outdated Show resolved Hide resolved
index.rst Outdated Show resolved Hide resolved
@jednano
Copy link
Member Author

jednano commented Nov 11, 2019

I've again addressed all the comments as best as I could in a8a5168.

@florianb
Copy link
Member

I'm fine with it. 😸

@jednano jednano merged commit ba41c75 into editorconfig:master Nov 12, 2019
@jednano jednano deleted the clarification branch November 12, 2019 06:16
@xuhdev
Copy link
Member

xuhdev commented Nov 12, 2019

🎆

@florianb
Copy link
Member

🎉

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