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

(#111) Advancing research for LORM implementation #184

Closed
wants to merge 1 commit into from

Conversation

llorllale
Copy link
Contributor

@llorllale llorllale commented Feb 14, 2018

as per #111

After doing some research it was decided we should go for Latent Semantic Indexing on the methods of the class in order to implement LORM. See this and this.

@0crat 0crat added the scope label Feb 14, 2018
@0crat
Copy link
Collaborator

0crat commented Feb 14, 2018

Job #184 is now in scope, role is REV

@codecov-io
Copy link

Codecov Report

Merging #184 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #184   +/-   ##
=========================================
  Coverage     74.79%   74.79%           
  Complexity      133      133           
=========================================
  Files            27       27           
  Lines           964      964           
  Branches         67       67           
=========================================
  Hits            721      721           
  Misses          213      213           
  Partials         30       30
Impacted Files Coverage Δ Complexity Δ
src/main/java/org/jpeek/App.java 93.87% <ø> (ø) 13 <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 501c18e...a0bbe42. Read the comment docs.

@0crat
Copy link
Collaborator

0crat commented Feb 14, 2018

This pull request #184 is assigned to @amihaiemil/z, here is why. The budget is 15 minutes, see §4. Please, read §27 and when you decide to accept the changes, inform @yegor256/z (the architect) right in this ticket. If you decide that this PR should not be accepted ever, also inform the architect.

@amihaiemil
Copy link
Contributor

amihaiemil commented Feb 15, 2018

@llorllale Looks ok, thanks. But there is a problem (not your fault). This research should have been done in the context of the ticket #160. Then, that puzzle should have been removed and replaced with the one you added here.

The task you had now was to implement LORM and use it in reporting. And it is stated that this task has impediments -- I'm not sure what to do in this situation? You could just instruct 0crat to wait, but then you would have a blocked task in your agenda for possibly a long time.

Anyway, quite complex, 0crat needs improvements still, to know which task to assign and which not...

@amihaiemil
Copy link
Contributor

@rultor goog to merge

@rultor
Copy link
Collaborator

rultor commented Feb 15, 2018

@rultor goog to merge

@amihaiemil Thanks for your request. @yegor256 Please confirm this.

@llorllale
Copy link
Contributor Author

@amihaiemil

And it is stated that this task has impediments -- I'm not sure what to do in this situation?

See comments starting at this one

@amihaiemil
Copy link
Contributor

@llorllale I see, ok :)

@amihaiemil
Copy link
Contributor

@yegor256 ping

@yegor256
Copy link
Member

@rultor merge

@rultor
Copy link
Collaborator

rultor commented Feb 16, 2018

@rultor merge

@yegor256 OK, I'll try to merge now. You can check the progress of the merge here

@rultor
Copy link
Collaborator

rultor commented Feb 16, 2018

@rultor merge

@llorllale @yegor256 Oops, I failed. You can see the full log here (spent 6min)

Tests run: 1, Failures: 0, Errors: 0, Skipped: 1, Time elapsed: 0 sec - in org.jpeek.web.ResultsTest
Running org.jpeek.IndexTest
Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 19.511 sec - in org.jpeek.IndexTest
Running org.jpeek.DefaultBaseTest
Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.05 sec - in org.jpeek.DefaultBaseTest

Results :

Tests run: 76, Failures: 0, Errors: 0, Skipped: 3

[INFO] 
[INFO] --- jacoco-maven-plugin:0.7.8:check (jacoco-check) @ jpeek ---
[INFO] Loading execution data file /home/r/repo/target/jacoco.exec
[INFO] Analyzed bundle 'jpeek' with 32 classes
[WARNING] Rule violated for bundle jpeek: lines covered ratio is 0.62, but expected minimum is 0.67
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 03:14 min
[INFO] Finished at: 2018-02-16T17:25:31+00:00
[INFO] Final Memory: 32M/208M
[INFO] ------------------------------------------------------------------------
[ERROR] Failed to execute goal org.jacoco:jacoco-maven-plugin:0.7.8:check (jacoco-check) on project jpeek: Coverage checks have not been met. See log for details. -> [Help 1]
org.apache.maven.lifecycle.LifecycleExecutionException: Failed to execute goal org.jacoco:jacoco-maven-plugin:0.7.8:check (jacoco-check) on project jpeek: Coverage checks have not been met. See log for details.
	at org.apache.maven.lifecycle.internal.MojoExecutor.execute(MojoExecutor.java:212)
	at org.apache.maven.lifecycle.internal.MojoExecutor.execute(MojoExecutor.java:153)
	at org.apache.maven.lifecycle.internal.MojoExecutor.execute(MojoExecutor.java:145)
	at org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject(LifecycleModuleBuilder.java:116)
	at org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject(LifecycleModuleBuilder.java:80)
	at org.apache.maven.lifecycle.internal.builder.singlethreaded.SingleThreadedBuilder.build(SingleThreadedBuilder.java:51)
	at org.apache.maven.lifecycle.internal.LifecycleStarter.execute(LifecycleStarter.java:128)
	at org.apache.maven.DefaultMaven.doExecute(DefaultMaven.java:307)
	at org.apache.maven.DefaultMaven.doExecute(DefaultMaven.java:193)
	at org.apache.maven.DefaultMaven.execute(DefaultMaven.java:106)
	at org.apache.maven.cli.MavenCli.execute(MavenCli.java:863)
	at org.apache.maven.cli.MavenCli.doMain(MavenCli.java:288)
	at org.apache.maven.cli.MavenCli.main(MavenCli.java:199)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)
	at org.codehaus.plexus.classworlds.launcher.Launcher.launchEnhanced(Launcher.java:289)
	at org.codehaus.plexus.classworlds.launcher.Launcher.launch(Launcher.java:229)
	at org.codehaus.plexus.classworlds.launcher.Launcher.mainWithExitCode(Launcher.java:415)
	at org.codehaus.plexus.classworlds.launcher.Launcher.main(Launcher.java:356)
Caused by: org.apache.maven.plugin.MojoExecutionException: Coverage checks have not been met. See log for details.
	at org.jacoco.maven.CheckMojo.executeCheck(CheckMojo.java:186)
	at org.jacoco.maven.CheckMojo.executeMojo(CheckMojo.java:160)
	at org.jacoco.maven.AbstractJacocoMojo.execute(AbstractJacocoMojo.java:63)
	at org.apache.maven.plugin.DefaultBuildPluginManager.executeMojo(DefaultBuildPluginManager.java:134)
	at org.apache.maven.lifecycle.internal.MojoExecutor.execute(MojoExecutor.java:207)
	... 20 more
[ERROR] 
[ERROR] Re-run Maven using the -X switch to enable full debug logging.
[ERROR] 
[ERROR] For more information about the errors and possible solutions, please read the following articles:
[ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoExecutionException
container 2945bac9338bda74b3e55d5efa16446cecf7119e2ceff0ea012d50084f9dbd04 is dead
Fri Feb 16 18:26:21 CET 2018

@llorllale
Copy link
Contributor Author

@amihaiemil any advice on how to fix this build error? How can I have changed the coverage when all I modified were an XSL and removed a comment in App.java?

@amihaiemil
Copy link
Contributor

@llorllale as the logs say, it fails because jacoco detected a lower test coverage than allowed: Rule violated for bundle jpeek: lines covered ratio is 0.62, but expected minimum is 0.67

Yes, your PR did not alter code coverage. Maybe it was changed in an earlier PR which was merged before this.

I would say: open a ticket about this, instruct 0crat to wait on your task and close this PR in the meantime (but don't delete your branch so you can make another PR after that ticket is solved).

@amihaiemil
Copy link
Contributor

amihaiemil commented Feb 17, 2018

@llorllale It also happened here: #179

And it will most likely happen with all the open PRs.

@llorllale
Copy link
Contributor Author

llorllale commented Feb 17, 2018

@amihaiemil Closing while waiting on #192

@llorllale llorllale closed this Feb 17, 2018
@0crat
Copy link
Collaborator

0crat commented Feb 17, 2018

@ypshenychka/z please review this job, as in §30

@llorllale
Copy link
Contributor Author

llorllale commented Feb 17, 2018

@llorllale

0crat must be having a hangover :) my bad, just read the policy for QA

Carry on!

@amihaiemil
Copy link
Contributor

@llorllale yes, I guess in the case of PRs, the QA reviews the job done by the CR. I hope I did the right thing here, advised you properly (that's what seemed logical to me).

@llorllale
Copy link
Contributor Author

llorllale commented Feb 18, 2018

@amihaiemil FYI just opened #197

@0crat
Copy link
Collaborator

0crat commented Feb 19, 2018

@amihaiemil/z this job was assigned to you 5 days ago. It will be taken away from you soon, unless you close it, see §8. Read this and this, please.

@ypshenychka
Copy link

@llorllale According to our QA Rules:

  1. Pull request description explains the solution proposed and contains a link to the original ticket it is related to.

Please provide more details in description.

  1. Messages in a ticket always start with a name of a user they are addressed to.

Please correct your messages by indicating an addressee in the beginning.

@ypshenychka
Copy link

@amihaiemil According to our QA Rules:

The code reviewer found at least three problems in the code.

No issues were found during code review.
Please confirm that you'll try to find at least three problems while future reviews.

@amihaiemil
Copy link
Contributor

@ypshenychka I confirm I'll try to find more issues in future tasks

@llorllale
Copy link
Contributor Author

@ypshenychka fixed description and my messages

@ypshenychka
Copy link

@llorllale Thank you!

@ypshenychka
Copy link

@0crat quality acceptable

@0crat
Copy link
Collaborator

0crat commented Feb 20, 2018

QA review completed: +15 points just awarded to @ypshenychka/z, total is +345

@0crat
Copy link
Collaborator

0crat commented Feb 20, 2018

Order was finished, quality was "acceptable": +15 points just awarded to @amihaiemil/z, total is +805

@0crat
Copy link
Collaborator

0crat commented Feb 20, 2018

Payment to ARC for a closed pull request, as in §28: +15 points just awarded to @yegor256/z, total is +10349

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants