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

Add .editorconfig to ease contribution to the project #248

Closed
MunchyYDL opened this Issue Aug 30, 2017 · 9 comments

Comments

Projects
None yet
6 participants
@MunchyYDL
Contributor

MunchyYDL commented Aug 30, 2017

I think it would be a good idea to add a .editorconfig file with some of the projects code styles configured. As this would potentially minimise the amount of formatting errors in future PRs. :)

@StanleyGoldman

This comment has been minimized.

Show comment
Hide comment
@StanleyGoldman

StanleyGoldman Aug 30, 2017

Member

I agree, an .editorconfig is probably a good thing.

I always forget that .editorconfig is just the basic stuff like tabs/spaces and tab/space width and such. I wish there were something really cool for code formatting, but that only always just get's you 4/5's of the way there.

Feel free to take this on if you like.

Member

StanleyGoldman commented Aug 30, 2017

I agree, an .editorconfig is probably a good thing.

I always forget that .editorconfig is just the basic stuff like tabs/spaces and tab/space width and such. I wish there were something really cool for code formatting, but that only always just get's you 4/5's of the way there.

Feel free to take this on if you like.

@MunchyYDL

This comment has been minimized.

Show comment
Hide comment
@MunchyYDL

MunchyYDL Aug 30, 2017

Contributor

Sure, I will grab this and get an initial configuration in place. :)

Contributor

MunchyYDL commented Aug 30, 2017

Sure, I will grab this and get an initial configuration in place. :)

@garg000dhruv

This comment has been minimized.

Show comment
Hide comment
@garg000dhruv

garg000dhruv Oct 4, 2017

In the spirit of HacktoberFest, I can pick this up. Let me know if you guys still need it.

garg000dhruv commented Oct 4, 2017

In the spirit of HacktoberFest, I can pick this up. Let me know if you guys still need it.

@idealadarsh

This comment has been minimized.

Show comment
Hide comment
@idealadarsh

idealadarsh Oct 8, 2017

@leereilly @StanleyGoldman I'd like to work on this issue.

idealadarsh commented Oct 8, 2017

@leereilly @StanleyGoldman I'd like to work on this issue.

@StanleyGoldman

This comment has been minimized.

Show comment
Hide comment
@StanleyGoldman

StanleyGoldman Oct 9, 2017

Member

Cool beans man.

Member

StanleyGoldman commented Oct 9, 2017

Cool beans man.

@Frozenfire92

This comment has been minimized.

Show comment
Hide comment
@Frozenfire92

Frozenfire92 Mar 21, 2018

Contributor

Was looking at #350 and it seems it is on hold and the repository deleted. Having some experience with editorconfig files I hope to help clear up some confusion and start some discussion.

From the website:

EditorConfig helps developers define and maintain consistent coding styles between different editors and IDEs. The EditorConfig project consists of a file format for defining coding styles and a collection of text editor plugins that enable editors to read the file format and adhere to defined styles. EditorConfig files are easily readable and they work nicely with version control systems.

An analogy from many javascript projects (not sure c# equivalents)

  • EditorConfig sets the text editors tab/spaces, width, etc when working on this project
  • eslint, jshint, etc are tools for validating rules (with optional editor plugins), which could also test for EditorConfig rules

From what I can tell, this projects styleguide most of these rules would fall into the linter category, which I think is beyond the scope of EditorConfig.

It seems many of the options in #350 come from Visual Studio 2017's integration reference and potentially this article

However those rules may be specific to VS2017. I suspect many developers will use that as its Unity's default, but I wonder about other editors support, and if maintaining these rules is worth the effort for something that is meant to make things simple and consistent. I would assume that many editor's will support the c# solution, so this list should be improved / tested against other editors instead of using single editor specific properties.

I would propose we take a simpler approach to editorconfig like so

EditorConfig property reference

# http://EditorConfig.org

root = true

[*]
indent_size = 4
indent_style = space
insert_final_newline = true
trim_trailing_whitespace = true
Contributor

Frozenfire92 commented Mar 21, 2018

Was looking at #350 and it seems it is on hold and the repository deleted. Having some experience with editorconfig files I hope to help clear up some confusion and start some discussion.

From the website:

EditorConfig helps developers define and maintain consistent coding styles between different editors and IDEs. The EditorConfig project consists of a file format for defining coding styles and a collection of text editor plugins that enable editors to read the file format and adhere to defined styles. EditorConfig files are easily readable and they work nicely with version control systems.

An analogy from many javascript projects (not sure c# equivalents)

  • EditorConfig sets the text editors tab/spaces, width, etc when working on this project
  • eslint, jshint, etc are tools for validating rules (with optional editor plugins), which could also test for EditorConfig rules

From what I can tell, this projects styleguide most of these rules would fall into the linter category, which I think is beyond the scope of EditorConfig.

It seems many of the options in #350 come from Visual Studio 2017's integration reference and potentially this article

However those rules may be specific to VS2017. I suspect many developers will use that as its Unity's default, but I wonder about other editors support, and if maintaining these rules is worth the effort for something that is meant to make things simple and consistent. I would assume that many editor's will support the c# solution, so this list should be improved / tested against other editors instead of using single editor specific properties.

I would propose we take a simpler approach to editorconfig like so

EditorConfig property reference

# http://EditorConfig.org

root = true

[*]
indent_size = 4
indent_style = space
insert_final_newline = true
trim_trailing_whitespace = true
@StanleyGoldman

This comment has been minimized.

Show comment
Hide comment
@StanleyGoldman

StanleyGoldman Mar 27, 2018

Member

Hey @Frozenfire92 thanks for participating.

Yea, I think a large part of the problem in #350 was that it was a bit too much too soon. (I just closed that pull request.) It seemed like the pull request was attempting to add a large template without regard to the code styles we value in this project.

The other part of the problem is there would not be a uniform way of applying the editor styles in both Visual Studio 2015 and Visual Studio 2017.

Something that you proposed that should work a bit more universally would be appreciated. If you want to put that in a pull request I can see about getting that merged in.

Member

StanleyGoldman commented Mar 27, 2018

Hey @Frozenfire92 thanks for participating.

Yea, I think a large part of the problem in #350 was that it was a bit too much too soon. (I just closed that pull request.) It seemed like the pull request was attempting to add a large template without regard to the code styles we value in this project.

The other part of the problem is there would not be a uniform way of applying the editor styles in both Visual Studio 2015 and Visual Studio 2017.

Something that you proposed that should work a bit more universally would be appreciated. If you want to put that in a pull request I can see about getting that merged in.

@Frozenfire92

This comment has been minimized.

Show comment
Hide comment
@Frozenfire92

Frozenfire92 Apr 7, 2018

Contributor

After finally going to do this I noticed its already in the project

https://github.com/github-for-unity/Unity/blob/master/.editorconfig

Unless the two options I proposed above are desired this could likely be closed

insert_final_newline = true
trim_trailing_whitespace = true
Contributor

Frozenfire92 commented Apr 7, 2018

After finally going to do this I noticed its already in the project

https://github.com/github-for-unity/Unity/blob/master/.editorconfig

Unless the two options I proposed above are desired this could likely be closed

insert_final_newline = true
trim_trailing_whitespace = true
@StanleyGoldman

This comment has been minimized.

Show comment
Hide comment
@StanleyGoldman

StanleyGoldman Apr 9, 2018

Member

😓... oh... sorry about that... i forgot..

Those two settings sound positive to me
Send it in a pull request and we can close this issue out.

Member

StanleyGoldman commented Apr 9, 2018

😓... oh... sorry about that... i forgot..

Those two settings sound positive to me
Send it in a pull request and we can close this issue out.

Frozenfire92 added a commit to Frozenfire92/Unity that referenced this issue Apr 14, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment