Skip to content

Conversation

Vampire
Copy link
Member

@Vampire Vampire commented Jul 9, 2020

This change is Reviewable

Copy link
Member

@leonard84 leonard84 left a comment

Choose a reason for hiding this comment

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

I don't see a reason to make this repeatable, what additional functionality do we get here?

@codecov
Copy link

codecov bot commented Jul 9, 2020

Codecov Report

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

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #1189   +/-   ##
=========================================
  Coverage     75.96%   75.96%           
  Complexity     3647     3647           
=========================================
  Files           393      393           
  Lines         11113    11113           
  Branches       1369     1369           
=========================================
  Hits           8442     8442           
  Misses         2192     2192           
  Partials        479      479           

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 3370650...ada0f07. Read the comment docs.

@Vampire Vampire force-pushed the repeatable-issue-annotation branch from 1fe48f7 to fcecd80 Compare July 9, 2020 16:54
@Vampire
Copy link
Member Author

Vampire commented Jul 9, 2020

I don't see a reason to make this repeatable, what additional functionality do we get here?

Same as for some of the others.
Being able to instead of

@Issue(["http://my.issues.org/FOO-1", "http://my.issues.org/FOO-2"])

write

@Issue("http://my.issues.org/FOO-1")
@Issue("http://my.issues.org/FOO-2")

This makes it better commentable if necessary,
better copyable if necessary,
subjectively better readable for some,
semantically better matching (it is @Issue, not @Issues)
...

Imho most of these should have been repeatable right from the start.
But then it was not possible, so an array value was the next best choice for multiple values.
And now having both allows old code to continue to work and new code to improve.
Or for some of the annotations like @Use maybe to group some values but not others.

@Vampire Vampire force-pushed the repeatable-issue-annotation branch from fcecd80 to b59b369 Compare July 9, 2020 23:19
@Vampire Vampire force-pushed the repeatable-issue-annotation branch from b59b369 to ada0f07 Compare July 13, 2020 19:41
@leonard84 leonard84 merged commit f3aefd7 into spockframework:master Jul 14, 2020
@Vampire Vampire deleted the repeatable-issue-annotation branch July 14, 2020 14:37
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.

2 participants