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

How to help with Astyle #22093

Closed
angoddu opened this Issue Oct 6, 2017 · 4 comments

Comments

Projects
None yet
4 participants
@angoddu
Contributor

angoddu commented Oct 6, 2017

I would like to do more to help this game develop as I really love playing it. While I'm learning the ins and outs of how the code functions, I figured I could start by helping with the Astyle stuff so here are my questions.

What needs to get done?
How many files should I re-style in a single PR?
Is it just the JSON files that need re-styling or does the C++ code also need re-styling?

I wish I could help with the translation stuff but I don't know another (spoken :P) language. I would love to be able to work on the little fixes so people like @codemime and @Night-Pryanik can focus on the bigger picture things. I know C++ fairly well so I can make fixes to the source code as well as fixes to the JSON files.

@kevingranade

This comment has been minimized.

Show comment
Hide comment
@kevingranade

kevingranade Oct 6, 2017

Member

I'd hold off on json styling for the moment, I'm just about to swap out the tool we use to do that with #21612, and it will work slightly differently after that lands.

For c++ styling, we use the astyle tool, and have put some wrappers around it in Makefile.
I have a brief outline of a batch settling workflow in doc/CODE_STYLE.md, feel free to ask for clarification on that since I don't know of anyone doing it except myself.

Generally though, you want to restyle a manageable chunk of code, read the diff to check for obvious errors, build the game (with RELEASE=1) and run it to again check for fairly obvious errors, remove any fully-styled files from the astyle blacklist file, then make a pull request with your changes.

Member

kevingranade commented Oct 6, 2017

I'd hold off on json styling for the moment, I'm just about to swap out the tool we use to do that with #21612, and it will work slightly differently after that lands.

For c++ styling, we use the astyle tool, and have put some wrappers around it in Makefile.
I have a brief outline of a batch settling workflow in doc/CODE_STYLE.md, feel free to ask for clarification on that since I don't know of anyone doing it except myself.

Generally though, you want to restyle a manageable chunk of code, read the diff to check for obvious errors, build the game (with RELEASE=1) and run it to again check for fairly obvious errors, remove any fully-styled files from the astyle blacklist file, then make a pull request with your changes.

@angoddu

This comment has been minimized.

Show comment
Hide comment
@angoddu

angoddu Oct 6, 2017

Contributor

If I'm reading this right then I could just install the sublime style, open the cpp files, auto format them, test, then remove them from the black list. Is that all that would need to be done or am I missing something

Contributor

angoddu commented Oct 6, 2017

If I'm reading this right then I could just install the sublime style, open the cpp files, auto format them, test, then remove them from the black list. Is that all that would need to be done or am I missing something

@ZhilkinSerg

This comment has been minimized.

Show comment
Hide comment
@ZhilkinSerg

ZhilkinSerg Oct 6, 2017

Contributor

You also need compile and run game after you astyled them.

Contributor

ZhilkinSerg commented Oct 6, 2017

You also need compile and run game after you astyled them.

@angoddu

This comment has been minimized.

Show comment
Hide comment
@angoddu

angoddu Oct 8, 2017

Contributor

Right. That's what I would do when I test the changes

Contributor

angoddu commented Oct 8, 2017

Right. That's what I would do when I test the changes

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