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

150 update implemented metrics readme #435

Merged

Conversation

victornoel
Copy link
Contributor

@victornoel victornoel commented Mar 30, 2020

This is for #150

I tracked the references in the tickets where the metrics were introduced, and since some of them had no available pdf on the web, I decided to directly link the pdf in this repository instead.

So I applied the same to all other metrics as well as fixed markdown and other errors.

The README can be seen rendered here: https://github.com/victornoel/jpeek/tree/150-update-implemented-metrics-readme

@0crat
Copy link
Collaborator

0crat commented Mar 30, 2020

This pull request #435 is assigned to @fanifieiev/z, here is why; the budget is 15 minutes, see §4; please, read §27 and when you decide to accept the changes, inform @paulodamaso/z (the architect) right in this ticket; if you decide that this PR should not be accepted ever, also inform the architect; this blog post will help you understand what is expected from a code reviewer; there will be a monetary reward for this job

@fanifieiev
Copy link

@0crat refuse

@0crat
Copy link
Collaborator

0crat commented Mar 30, 2020

@0crat refuse (here)

@fanifieiev The user @fanifieiev/z resigned from #435, please stop working. Reason for job resignation: Order was cancelled

@0crat
Copy link
Collaborator

0crat commented Mar 30, 2020

@0crat refuse (here)

@fanifieiev Job refused in 0 hours - no penalty, see §6

@0crat
Copy link
Collaborator

0crat commented Mar 30, 2020

This pull request #435 is assigned to @Vatavuk/z, here is why; the budget is 15 minutes, see §4; please, read §27 and when you decide to accept the changes, inform @paulodamaso/z (the architect) right in this ticket; if you decide that this PR should not be accepted ever, also inform the architect; this blog post will help you understand what is expected from a code reviewer; there will be a monetary reward for this job

@Vatavuk
Copy link

Vatavuk commented Mar 30, 2020

@paulodamaso should we close #433 ?

@Vatavuk
Copy link

Vatavuk commented Mar 31, 2020

@paulodamaso ping

@paulodamaso
Copy link
Collaborator

@Vatavuk Please go on with this review, I've closed #433

Copy link

@Vatavuk Vatavuk left a comment

Choose a reason for hiding this comment

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

@victornoel one comment

README.md Outdated
@@ -110,84 +110,105 @@ _Revisiting Class Cohesion: An empirical investigation on several systems_,<br/>
Journal of Object Technology, vol.7, no.6, 2008,
[PDF](http://www.jot.fm/issues/issue_2008_07/article1.pdf).

Here is a list of metrics we already implement:
### Here is a list of metrics we already implement:
Copy link

Choose a reason for hiding this comment

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

@victornoel I believe it should be we've already implemented

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Vatavuk both are correct english, but I will change, it is same the for me :)

@victornoel
Copy link
Contributor Author

@Vatavuk thanks, it is done

Copy link

@Vatavuk Vatavuk left a comment

Choose a reason for hiding this comment

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

@paulodamaso good to merge

@paulodamaso
Copy link
Collaborator

@victornoel build is failing (?) please take a look

@victornoel
Copy link
Contributor Author

@paulodamaso clearly this is not related to the content of this PR, there seem to be an error with the maven build… maybe it is a transient error? Could you try re-trigger the build?

@paulodamaso
Copy link
Collaborator

@rultor merge

@rultor
Copy link
Collaborator

rultor commented Apr 1, 2020

@rultor merge

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

@rultor rultor merged commit 2a5a27c into cqfn:master Apr 1, 2020
@rultor
Copy link
Collaborator

rultor commented Apr 1, 2020

@rultor merge

@paulodamaso Done! FYI, the full log is here (took me 16min)

@victornoel victornoel deleted the 150-update-implemented-metrics-readme branch April 1, 2020 18:15
@0crat
Copy link
Collaborator

0crat commented Apr 1, 2020

@sereshqua/z please review this job completed by @Vatavuk/z, as in §30; the job will be fully closed and all payments will be made when the quality review is completed

@0crat 0crat removed the scope label Apr 1, 2020
@sereshqua
Copy link

@Vatavuk please make sure you will find at least 3 issues during next CR; also, keep in mind that these have to be design problems, but not rather cosmetic issues like localisation

@Vatavuk
Copy link

Vatavuk commented Apr 2, 2020

@sereshqua ok

@sereshqua
Copy link

@0crat quality acceptable

1 similar comment
@sereshqua
Copy link

@0crat quality acceptable

@victornoel
Copy link
Contributor Author

@sereshqua actually it worked, it is as @g4s8 said in https://github.com/zerocracy/farm/issues/2252, it should be fixed the next time you get a QA job :)

@sereshqua
Copy link

@victornoel do you mean QA label or?

should NOT we see smth like this already?

Screenshot 2020-04-03 at 12 34 19

@0crat
Copy link
Collaborator

0crat commented Apr 3, 2020

@0crat quality acceptable (here)

@sereshqua There is no quality review for #435, no performer

@victornoel
Copy link
Contributor Author

@victornoel do you mean QA label or?

should NOT we see smth like this already?

Screenshot 2020-04-03 at 12 34 19

@sereshqua well, not here, because the QA here happened before the fix was merged

@sereshqua
Copy link

@victornoel look at above, I think now it is working properly :D

@victornoel
Copy link
Contributor Author

@sereshqua ah yes, very cool :)

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

Successfully merging this pull request may close these issues.

None yet

7 participants