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

#184 Setup Code Style Checks 📦 #188

Merged
merged 46 commits into from Apr 30, 2020
Merged

#184 Setup Code Style Checks 📦 #188

merged 46 commits into from Apr 30, 2020

Conversation

roma-glushko
Copy link
Member

@roma-glushko roma-glushko commented Apr 20, 2020

Description

This PR introduces static code checks:

2020-04-21_00-03-19

Fixed Issues

Contribution checklist

  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages

@roma-glushko roma-glushko changed the title #184 Setup Code Style Checks #184 Setup Code Style Checks 📦 Apr 20, 2020
@roma-glushko roma-glushko linked an issue Apr 20, 2020 that may be closed by this pull request
@VitaliyBoyko
Copy link
Contributor

Let's temporarily suppress all the existing .java files. What do you think?

@coderimus
Copy link
Contributor

@VitaliyBoyko I think that we can get more benefits if all style checks are fixed in the scope of this PR. The 1.0.1 release, as I can see, has a lot of new feature requests and while they are in progress we can adopt already existed code base to the best practices according to standads.

@roma-glushko maybe we have something similar to the phpcs fixer in the Java world :) if yes we can automatically fix tons of warnings such as spaces and general formatting issue.

If we can split code style issues between us I am ready to help.

@VitaliyBoyko
Copy link
Contributor

VitaliyBoyko commented Apr 21, 2020

@coderimus Got you point, let me create kinda' epic branch. Then we will able to make PRs there.

@VitaliyBoyko
Copy link
Contributor

However, I'm afraid we break something during refactoring.

@VitaliyBoyko
Copy link
Contributor

VitaliyBoyko commented Apr 21, 2020

Maybe it better to fix it in small portions 🤔 And adding test coverage as well.

@VitaliyBoyko VitaliyBoyko changed the base branch from 1.0.1-develop to 184-code-styling-epic April 21, 2020 08:29
@VitaliyBoyko
Copy link
Contributor

Here we go 184-code-styling-epic

@coderimus
Copy link
Contributor

@VitaliyBoyko I like that idea about the small portions and test coverage. What about granulated work per action? This will allow making the process more organized and connect other project members.

However, I'm afraid we break something during refactoring.

Yep. I think we will find and fix already existed bugs :)

@VitaliyBoyko
Copy link
Contributor

Let's discuss it at a meeting. I see some additional points to cover.

@VitaliyBoyko VitaliyBoyko self-assigned this Apr 21, 2020
@m2-community-project m2-community-project bot moved this from Ready for Review to Review in Progress in Pull Request Progress Apr 21, 2020
@roma-glushko
Copy link
Member Author

roma-glushko commented Apr 21, 2020

@VitaliyBoyko @coderimus thank you for your attention to this PR 🙌

At this point, we can see that there are still a couple of active static check projects in Java we can try to use 😄

However, each of these three tools reports 2k+ warnings/errors which we can not fix over the night. So my next steps here are:

  • review issues that each of the tool reports to see whether they make sense for the project
  • reduce checking scope to PR changeset only (otherwise, we will break every next PR in the project)
  • try to fix at least code style issues somehow automatically

I will proceed working on this draft and let you know when I investigate the mentioned points.

@VitaliyBoyko VitaliyBoyko changed the base branch from 184-code-styling-epic to 1.0.1-develop April 21, 2020 18:43
@VitaliyBoyko
Copy link
Contributor

Changed back to 1.0.1-develop according to our personal discussion.

@VitaliyBoyko VitaliyBoyko self-requested a review April 30, 2020 17:17
@VitaliyBoyko VitaliyBoyko marked this pull request as ready for review April 30, 2020 17:35
@VitaliyBoyko VitaliyBoyko merged commit 2f254fc into magento:1.0.1-develop Apr 30, 2020
@m2-community-project m2-community-project bot moved this from Review in Progress to Done in Pull Request Progress Apr 30, 2020
@roma-glushko roma-glushko deleted the 184-setup-code-style-checks branch June 25, 2020 07:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Award: advanced Award: category of expertise Award: complex infrastructure Partner: Atwix partners-contribution Priority: P1 Needs to be fixed before any other issues Severity: S1 Affects critical data or functionality and forces users to employ a workaround
Projects
Development

Successfully merging this pull request may close these issues.

Setup Code Style Checks
4 participants