Skip to content

Conversation

llorllale
Copy link
Contributor

This PR is for #154:

  • Adds filtering capability for ListedImages according to the docs.

This PR took me way longer than it looks. The documentation referenced above is not entirely clear, and searches for some examples yield filters not included in the documentation(!) (1, 2).

I have not tested this solution against a live docker instance, and I'm not quite satisfied both with the efficiency of the unit test I wrote, nor with the elegance of both the implementation and test. ¯\_(ツ)_/¯ Maybe we can add a puzzle for that. Maybe we can also wait for #153 and make sure this actually works when we implement integration tests for that.

@coveralls
Copy link

coveralls commented Sep 29, 2018

Pull Request Test Coverage Report for Build 297

  • 14 of 14 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.5%) to 81.653%

Totals Coverage Status
Change from base Build 295: 0.5%
Covered Lines: 405
Relevant Lines: 496

💛 - Coveralls

@0crat
Copy link
Collaborator

0crat commented Sep 29, 2018

Job #159 is now in scope, role is REV

@0crat
Copy link
Collaborator

0crat commented Sep 29, 2018

This pull request #159 is assigned to @llorllale/z, here is why; the budget is 15 minutes, see §4; please, read §27 and when you decide to accept the changes, inform @amihaiemil/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

@llorllale
Copy link
Contributor Author

@amihaiemil I can't be REV :)

@amihaiemil
Copy link
Owner

@llorllale well, looks ok. We'll see about integration testing, later. I boosted the original task 6x now, that is the maximum according to the Policy :D

@amihaiemil
Copy link
Owner

@rultor merge it

@rultor
Copy link
Collaborator

rultor commented Oct 3, 2018

@rultor merge it

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

@rultor rultor merged commit 1b73a18 into amihaiemil:master Oct 3, 2018
@rultor
Copy link
Collaborator

rultor commented Oct 3, 2018

@rultor merge it

@amihaiemil Done! FYI, the full log is here (took me 3min)

@0crat
Copy link
Collaborator

0crat commented Oct 3, 2018

@amihaiemil/z everybody who has role REV is banned at #159; I won't be able to assign anyone automatically; consider assigning someone manually (as in §19), or invite more people (as in §51), or remove the job from the scope (as in §14)

@0crat
Copy link
Collaborator

0crat commented Oct 3, 2018

Order was finished: +15 point(s) just awarded to @llorllale/z

@0crat
Copy link
Collaborator

0crat commented Oct 3, 2018

The job #159 is now out of scope

@0crat
Copy link
Collaborator

0crat commented Oct 3, 2018

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

@llorllale llorllale deleted the 154 branch October 9, 2018 00:17
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.

5 participants