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

README documentation for SetOf was added #1250

Merged
merged 3 commits into from
Dec 3, 2019
Merged

Conversation

fanifieiev
Copy link
Contributor

This PR is for issue 1246.
This includes an additional documentation description for SetOf.

@0crat
Copy link
Collaborator

0crat commented Nov 26, 2019

Job #1250 is now in scope, role is REV

@0crat 0crat added the scope label Nov 26, 2019
@0crat
Copy link
Collaborator

0crat commented Nov 26, 2019

This pull request #1250 is assigned to @iakunin/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 no monetary reward for this job

Copy link
Contributor

@iakunin iakunin left a comment

Choose a reason for hiding this comment

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

@fanifieiev just a few cosmetic changes. Could you have a look?

README.md Outdated
@@ -208,6 +208,33 @@ int total = new LengthOf(
).intValue();
```

To get a set of unique elements by providing varargs:
Copy link
Contributor

Choose a reason for hiding this comment

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

@fanifieiev To get a set sounds a little bit vague, in my opinion. Maybe it's better to start with To create a set?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@fanifieiev To get a set sounds a little bit vague, in my opinion. Maybe it's better to start with To create a set?

@iakunin Agree, will fix.

README.md Outdated
);
```

To get a set of unique elements from existing iterable:
Copy link
Contributor

Choose a reason for hiding this comment

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

@fanifieiev To get a set sounds a little bit vague, in my opinion. Maybe it's better to start with To create a set?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@iakunin Agree, will fix.

README.md Outdated
);
```

To get sorted iterable with unique elements from existing iterable:
Copy link
Contributor

Choose a reason for hiding this comment

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

@fanifieiev just by analogy with the previous sentences we can start this one with To create sorted iterable

README.md Outdated
@@ -208,6 +208,33 @@ int total = new LengthOf(
).intValue();
```

To get a set of unique elements by providing varargs:
Copy link
Contributor

Choose a reason for hiding this comment

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

@fanifieiev a phrase set of unique elements could be a little bit excessive one, because the definition of Set already implies that:

set is an abstract data type that can store unique values

@fanifieiev what do you think?

Copy link
Contributor Author

@fanifieiev fanifieiev Nov 28, 2019

Choose a reason for hiding this comment

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

@iakunin Agree.

README.md Outdated
);
```

To get a set of unique elements from existing iterable:
Copy link
Contributor

Choose a reason for hiding this comment

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

@fanifieiev a phrase set of unique elements could be a little bit excessive one, because the definition of Set already implies that:

set is an abstract data type that can store unique values

@fanifieiev what do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@iakunin Will fix that.

@codecov-io
Copy link

codecov-io commented Nov 28, 2019

Codecov Report

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

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #1250   +/-   ##
=========================================
  Coverage     89.32%   89.32%           
  Complexity     1686     1686           
=========================================
  Files           280      280           
  Lines          4028     4028           
  Branches        212      212           
=========================================
  Hits           3598     3598           
  Misses          396      396           
  Partials         34       34

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 95f5a04...002dbab. Read the comment docs.

@iakunin
Copy link
Contributor

iakunin commented Nov 28, 2019

@fanifieiev thank you for your fixes

@iakunin
Copy link
Contributor

iakunin commented Nov 28, 2019

@paulodamaso looks good to merge

@fanifieiev
Copy link
Contributor Author

@paulodamaso looks good to merge

@paulodamaso ping

@iakunin
Copy link
Contributor

iakunin commented Dec 2, 2019

@paulodamaso ping

@paulodamaso
Copy link
Contributor

@iakunin @fanifieiev Sorry for the delay, guys

@paulodamaso
Copy link
Contributor

@rultor merge

@rultor
Copy link
Collaborator

rultor commented Dec 3, 2019

@rultor merge

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

@rultor rultor merged commit 002dbab into yegor256:master Dec 3, 2019
@rultor
Copy link
Collaborator

rultor commented Dec 3, 2019

@rultor merge

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

@0crat
Copy link
Collaborator

0crat commented Dec 3, 2019

@sereshqua/z please review this job completed by @iakunin/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 Dec 3, 2019
@0crat
Copy link
Collaborator

0crat commented Dec 3, 2019

The job #1250 is now out of scope

@0crat
Copy link
Collaborator

0crat commented Dec 3, 2019

Payment to ARC for a closed pull request, as in §28: +10 point(s) just awarded to @paulodamaso/z

@sereshqua
Copy link

@iakunin please make you comments during next CR would be mostly about design problems, not just cosmetic issues

@iakunin
Copy link
Contributor

iakunin commented Dec 3, 2019

@sereshqua got it. I’ll try my best.

@sereshqua
Copy link

@0crat quality acceptable

@0crat
Copy link
Collaborator

0crat commented Dec 3, 2019

Order was finished, quality is "acceptable": +15 point(s) just awarded to @iakunin/z

@0crat
Copy link
Collaborator

0crat commented Dec 3, 2019

Quality review completed: +4 point(s) just awarded to @sereshqua/z

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.

7 participants