Closed dedece35 closed 1 year ago
Hi @zippy1978, please take into account my review comments in this PR.
I'm not expert in ios development but I've worked with java and maven since many years. If you want to discuss about my comments on Slack appointment, no problem. It will be a pleasure to talk to you.
Hi @julien-hertout-neomades, @jhertout, could you give me your test files to check that this new ios plugin is ok ? could we make an appointment on Slack / Meet to discuss about it ?
Hi @dedece35, And thanks for your feedbacks. I will have a look a them and probably get in touch with you when needed.
If possible, I will be interested to join the Slack as well !
hi @zippy1978 no problem to join slack : what's your email slack account to invite you ?
@dedece35 gi.grousset@gmail.com :)
HI @zippy1978, please also add the same modifications as https://github.com/green-code-initiative/ecoCode/pull/43/files
Hi @zippy1978 , I am joining @dedece35 to perform the review of the ios plugin PR. I let the maven configuration review to @dedece35 . I will more review the functionality itself. First of all I must say that the code is clean and clear, thanks.
I tried to run the plugin but I fail. I sent you a message on Slack on the subject.
I have some little points I would like to adjust or clarify:
I think that the documentation (Readme...) should be adjusted with what we have in the whole project but I think that we will do that after this first PR merge.
At this point it is all I have to say. May be I will have other questions \ remarks after I succeed to install the plugin.
Thanks
@jhertout @dedece35 , I did fix the packaging issue @jhertout had.
And also addressed the requested changes.
For the license header in files: I used this maven plugin (I used it on previous projects) : https://mycila.carbou.me/license-maven-plugin It can add and check source file headers automatically. Maybe something to add on other plugins as well ?
@jhertout
Regarding the stacktrace: it is not related to the ecoCode plugin but to the Swift lang support plugin (that is required) and that we have been developing at inside|app here: https://github.com/insideapp-oss/sonar-apple.
So we did some investigation, and it is probably caused by a mix in line ending chars in the source files. I think you did test on Windows, and your git client is probably configured to adapt line ending on checkout (and use CR+LF chars, instead of leaving LF - as used on macOS and Linux) as a result SonarQube cannot detect lines in source files.
On Windows this feature is called "autocrlf".
This problem is SonarQube related (not specific / not related to specific plugin) and can happen with any language (see https://stackoverflow.com/questions/43997878/sonarqube-c-sharp-analysis-fails-to-not-a-valid-line-offset-for-pointer)
Note that the issue does not appear on macOS and Linux (using your test project + running SonarQube in docker-compose).
Could you double check the line ending chars on your test project sources ?
Kudos, SonarCloud Quality Gate passed!
Hi @jhertout,
I addressed all the points you mentioned above.
The rule now looks like:
Hello @zippy1978 ,
it seems ok for me. I don't remember if I answer you relating the stacktrace I sent to you but you were right. The problem was that I work on Windows and my EOL was changed by git. I will try to perform a last test this week. If all is ok, I will merge this PR and we will follow the work in other PRs if necessary.
Thanks
Description
This PR added support to iOS mobile apps (with Swift language support).
In order to work, the plugin needs to be installed alongside a plugin supporting Swift as a language such as sonar-apple (open source) or the commercial version from SonarSource.
This initial version comes with a single rule, but lays the groundwork for implementing (many !) more. A basic CONTRIBUTION.md explains how to add new rules.
initial PR (before migration): https://github.com/cnumr/ecoCode/pull/146 (plop @zippy1978 )