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

Feature/phpstan #21

Merged
merged 6 commits into from
Mar 29, 2019
Merged

Feature/phpstan #21

merged 6 commits into from
Mar 29, 2019

Conversation

jeroenjans
Copy link
Contributor

This PR adds support for PHPStan using the CheckStyleParser.

@codecov
Copy link

codecov bot commented Mar 25, 2019

Codecov Report

Merging #21 into master will decrease coverage by 0.04%.
The diff coverage is 70%.

Impacted file tree graph

@@             Coverage Diff             @@
##             master     #21      +/-   ##
===========================================
- Coverage     80.84%   80.8%   -0.05%     
- Complexity     1371    1373       +2     
===========================================
  Files           225     226       +1     
  Lines          4684    4695      +11     
  Branches        375     375              
===========================================
+ Hits           3787    3794       +7     
- Misses          743     747       +4     
  Partials        154     154
Impacted Files Coverage Δ Complexity Δ
.../io/jenkins/plugins/analysis/warnings/PhpStan.java 70% <70%> (ø) 2 <2> (?)
...io/jenkins/plugins/analysis/warnings/YamlLint.java 37.5% <0%> (-5.36%) 0% <0%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update afb775e...1b1fcf2. Read the comment docs.

Copy link
Member

@uhafner uhafner left a comment

Choose a reason for hiding this comment

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

Is the icon also available under an OSS or community license? It can be integrated as well if you like...

public class PhpStan extends ReportScanningTool {
private static final long serialVersionUID = 2699509705079011738L;

static final String ID = "phpstan";
Copy link
Member

Choose a reason for hiding this comment

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

private

Copy link
Member

Choose a reason for hiding this comment

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

(I changed that in all other parsers as well:-)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've updated the visibility to private.
I've also rebased on master to resolve the merge conflict in the SUPPORTED-FORMATS.md file

@jeroenjans
Copy link
Contributor Author

Regarding the image I haven't been able to find information on the license of the icon.
I'll see if I can get some information from the developers on this.

@jeroenjans
Copy link
Contributor Author

Alright, I was able to check it with the developers and I was told that we can use the logo for this purpose.
I've added it in the last commit and updated the supported documents file.

@uhafner
Copy link
Member

uhafner commented Mar 28, 2019

Nice, thanks! Can you please add a line or reference to the icon license (or to the discussion) in the LICENSE.txt file in the icons folder?

@uhafner
Copy link
Member

uhafner commented Mar 28, 2019

And one more thing, can you add a small test case to ParsersITest that shows that your parser correctly finds n warnings. No need to check for individual properties, since this is done in the check style parser test.

@uhafner uhafner merged commit c4b608f into jenkinsci:master Mar 29, 2019
@uhafner
Copy link
Member

uhafner commented Mar 29, 2019

Thank you very much!

@jeroenjans jeroenjans deleted the feature/phpstan branch March 29, 2019 12:29
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.

2 participants