Skip to content
This repository was archived by the owner on Jan 14, 2025. It is now read-only.

Conversation

@scheglov
Copy link
Contributor

No description provided.

Copy link
Contributor

@devoncarew devoncarew left a comment

Choose a reason for hiding this comment

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

I like the idea of being able to have free-form text as a single positional param, and then being able to opt-into better toolability via named params.

@FailingTest('whoops')

@FailingTest('whoops', issue: 'http:/...')

instead of:

@FailingTest(reason: 'whoops', issue: 'http:/...')

As the first is slightly more terse when you just want to pass a reason. But I'm ok w/ landing either way, and seeing how it feels to use it.

*/
class FailingTest {
/**
* Initialize this annotation with the given issue URI.
Copy link
Contributor

Choose a reason for hiding this comment

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

with the given issue URI ==> with the given arguments

/**
* Initialize this annotation with the given issue URI.
*
* [issue] is a full URI describing the failure and used for tracking.
Copy link
Contributor

Choose a reason for hiding this comment

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

reason first, and then issue?

Choose a reason for hiding this comment

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

Alphabetic order lgtm.

Choose a reason for hiding this comment

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

If there is an unnamed parameter, and you only want to include an issue number, then you have to write:

@FailingTest('', issue: '...')

Which seems silly. I suspect that this is one of the reasons Flutter chose to make all parameters named, even when some are required,

/**
* Initialize this annotation with the given issue URI.
*
* [issue] is a full URI describing the failure and used for tracking.

Choose a reason for hiding this comment

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

Alphabetic order lgtm.

@scheglov scheglov merged commit f940644 into master Jul 26, 2018
@scheglov scheglov deleted the failingTest-named-parameters branch July 26, 2018 19:05
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Development

Successfully merging this pull request may close these issues.

5 participants