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

add some insert method and allow get mpqFile by blockTable #34

Merged
merged 12 commits into from
Apr 26, 2020

Conversation

PhoenixZeng
Copy link
Contributor

No description provided.

@coveralls
Copy link

coveralls commented Mar 25, 2020

Coverage Status

Coverage increased (+0.3%) to 85.928% when pulling 86d8a46 on PhoenixZeng:master into d782af0 on inwc3:master.

@Frotty
Copy link
Member

Frotty commented Mar 25, 2020

Thanks for your contribution. Please get rid of whitespace changes and add unit tests for the new features.

@Frotty
Copy link
Member

Frotty commented Mar 29, 2020

There are still some weird whitespace and formatting issues, other than that looks good.

@PhoenixZeng
Copy link
Contributor Author

#33

@Frotty
Copy link
Member

Frotty commented Apr 23, 2020

still has a bunch of formatting issues, otherwise looks fine ithink

@PhoenixZeng
Copy link
Contributor Author

please show me the formatting issue . I cant find it

@Frotty
Copy link
Member

Frotty commented Apr 23, 2020

ideally u should be using the IDEA formatter on ur changed code:
insertByteArray(name,input,false); -> insertByteArray(name, input, false);
mpqEditor.getMpqFilesByBlockTable().size()>0 -> mpqEditor.getMpqFilesByBlockTable().size() > 0
){ -> ) {
etc.

@PhoenixZeng
Copy link
Contributor Author

PhoenixZeng commented Apr 23, 2020

it`s hard for me. cause i changed the idea default setting.

@PhoenixZeng
Copy link
Contributor Author

I've tried my best. I don't want to change it if I still have format problems
I am willing to comply with most of the format requirements, such as indentation and common format requirements
Excessive format requirements will reduce other's desire for pr

@Frotty
Copy link
Member

Frotty commented Apr 23, 2020

How do you decide what's not common?
I think it is very basic that contributed code conforms to the standards of the existing code base.
I might check out the branch later.

@Crigges
Copy link
Member

Crigges commented Apr 23, 2020

Using IntelliJ's default autoformat uses spaces but is messing with the current JavaDoc format:
grafik

It seems like the "Do not wrap one line comments" under File | Settings | Editor | Code Style | Java | JavaDoc | Other seems to do the trick. For now, I've created a pull request for your fork. In the future, please restore the default IDEA Code Style settings (just for this project) and tic the checkbox mentioned before. Then autoformat your code using:

For Windows : Ctrl + Alt + L
For Ubuntu : Ctrl + Alt + Windows + L
For Mac : ⌥ (Option) + ⌘ (Command) + L

@Frotty
Copy link
Member

Frotty commented Apr 25, 2020

@Crigges use the function to "only format changed lines".

@Frotty
Copy link
Member

Frotty commented Apr 26, 2020

Thanks for your contribution 👍

@Frotty Frotty merged commit 7e87dc9 into inwc3:master Apr 26, 2020
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.

4 participants