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

Bring in Greg Allen's extensions and update to merge against current master #4

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

mrowe
Copy link

@mrowe mrowe commented Aug 19, 2014

This is basically Greg's changes as-is, tweaked to merge cleanly against deec9c4 (which incorporates 0e78ba0).

@mrowe
Copy link
Author

mrowe commented Aug 19, 2014

I have tested this with Sonar 3.7.4 and Sonar Runner 2.4 and it seems to work properly (with a very small sample size of one Rails-based project--I will extend this to a few more projects soon).

@mrowe
Copy link
Author

mrowe commented Aug 19, 2014

Ok, maybe I spoke too soon. It looks like it ran successfully, but when I look at the log I see a lot of this:

ERROR - Failed to register issue.
Issue Object : DefaultIssue[key=521df030-b76f-4302-8067-e92f73abef44,componentKey=com.rea-group:cp-agentadmin:image_migration/commercial_image_migration,projectKey=<null>,ruleKey=reek:IrresponsibleModule,severity=INFO,manualSeverity=false,message=has no descriptive comment,line=<null>,effortToFix=<null>,status=OPEN,resolution=<null>,reporter=<null>,assignee=<null>,checksum=<null>,attributes=<null>,authorLogin=<null>,actionPlanKey=<null>,comments=<null>,creationDate=<null>,updateDate=<null>,closeDate=<null>,currentChange=<null>,isNew=true,endOfLife=false,onDisabledRule=false,isChanged=false,sendNotifications=false,selectedAt=<null>]

Doing some more investigation now...

@mrowe
Copy link
Author

mrowe commented Aug 19, 2014

Ok, it looks like that was caused by Sonar picking up the wrong Quality Profile. Installing the new version of the plugin over the old one did not upgrade the existing Ruby profile, but created a new one (but it wasn't used because the old one remained the default).

I suspect this change is relevant? realestate-com-au@89568db#diff-533381a99017c42395810a97ae51afb2R3 (Specifically the "Sonar Way" --> "Sonar way" line).

@bsclifton
Copy link

We're finally pulling this in and testing it 😄 Sorry it took so long- we had tried it before but needed to upgrade to Sonar 3.7.4 LTS to try it out. We ran into problems and didn't revisit. We recently got another pull request and have more time to dedicate to this. Stay tuned...

@bsclifton
Copy link

@mrowe, not sure if you're still using this plugin (given how old this PR is; again, sorry for that).

We've got our Sonar instance upgraded to 3.7.4 and I've used the Sonar runner 2.4. When doing that, just like back in 2014 when we tried it, we are getting an error when the runner parses the metric_fu file. Any heads up on this is appreciated. Please see this gist for more info:
https://gist.github.com/bsclifton/37f79811abd21c51d584

We'll continue looking into this. If we can resolve the issue we'll merging this PR and start looking at the newer one for Sonar 4.5.2+.

@bsclifton
Copy link

Root cause for the issue we saw discovered; we had an older version of the metric_fu gem installed (4.4.1). We upgraded to the latest and this fixed the issue 👍

@bsclifton
Copy link

We pulled in #5 first which now makes this PR more complicated to merge in.

We should be able to work through the issues though and pull in the changes. Manual merge in progress...

bsclifton added a commit that referenced this pull request Sep 20, 2015
bsclifton added a commit that referenced this pull request Sep 20, 2015
bsclifton added a commit that referenced this pull request Sep 20, 2015
@mxsmith-godaddy
Copy link

We tried merging in this manually using the branch name: pr4-manual-merge. The package is complied, the tests are passing, however issues are not being reported. The root cause is the Issuable object uses an InputFile as its second parameter instead of the RubyFile object. Needs more work to leverage the new sonar api before the changes in this pull request will work. cc: @ggallen, @mrowe

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.

None yet

4 participants