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

✨ Assign issues to a designated assignee #17

Merged
merged 5 commits into from
Jun 27, 2023
Merged

✨ Assign issues to a designated assignee #17

merged 5 commits into from
Jun 27, 2023

Conversation

lig
Copy link
Contributor

@lig lig commented Jun 6, 2023

No description provided.

Copy link
Member

@samuelcolvin samuelcolvin left a comment

Choose a reason for hiding this comment

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

in general looks good.

src/logic/issues.py Outdated Show resolved Hide resolved
src/logic/issues.py Outdated Show resolved Hide resolved
def assign_new(self):
assignee = self._select_assignee()
self._assign_user(assignee)
self._add_label(self.config.unconfirmed_label)
Copy link
Member

Choose a reason for hiding this comment

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

what is the example use here of changing labels?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

A new issue gets unconfirmed label. After confirming contributor removes unconfirmed label.

There is a TODO in process_issue doc string suggesting adding a reaction on a magic comment like "can confirm" that should change unconfirmed label to confirmed.

src/settings.py Outdated Show resolved Hide resolved
@dataclass(kw_only=True)
class LabelAssign:
# for example "Selected Assignee: @samuelcolvin"
ASSIGNEE_REGEX: typing.ClassVar[re.Pattern] = re.compile(r'selected[ -]assignee:\s*@([\w\-]+)$', flags=re.I)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
ASSIGNEE_REGEX: typing.ClassVar[re.Pattern] = re.compile(r'selected[ -]assignee:\s*@([\w\-]+)$', flags=re.I)
ASSIGNEE_REGEX: typing.ClassVar[re.Pattern] = re.compile(r'selected[ -]assignee:\s*@([\w\-]+)$', flags=re.I)

I don't think this should necessarily have to end the line or all text.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, the issue may be edited by the author.

As your suggestion is empty I assume you are suggesting removing trailing $ from the regex. I would rather enable multiline and add leading ^. This will require the text to be on a dedicated line but will allow it to be in any place of the issue body.

BTW, this is just a copy of the reviewer regex from the sibling pr module.

Copy link
Member

Choose a reason for hiding this comment

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

agree requiring a new line is good, ideally we'd reuse the regex in both places.

lig and others added 2 commits June 6, 2023 20:38
@lig lig force-pushed the assign-issues branch 2 times, most recently from b80a7ee to 5ca7522 Compare June 9, 2023 11:55
@lig lig requested a review from samuelcolvin June 9, 2023 17:35
@lig lig assigned lig and samuelcolvin and unassigned lig Jun 9, 2023
@lig
Copy link
Contributor Author

lig commented Jun 9, 2023

please review :)

Copy link
Member

@samuelcolvin samuelcolvin left a comment

Choose a reason for hiding this comment

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

I haven't ready everything exactly but overall looking good.

src/logic/issues.py Show resolved Hide resolved
@dataclass(kw_only=True)
class LabelAssign:
# for example "Selected Assignee: @samuelcolvin"
ASSIGNEE_REGEX: typing.ClassVar[re.Pattern] = re.compile(r'selected[ -]assignee:\s*@([\w\-]+)$', flags=re.I)
Copy link
Member

Choose a reason for hiding this comment

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

agree requiring a new line is good, ideally we'd reuse the regex in both places.

@lig
Copy link
Contributor Author

lig commented Jun 20, 2023

please review

@lig lig requested a review from samuelcolvin June 20, 2023 14:10
@lig lig marked this pull request as ready for review June 26, 2023 13:22
Copy link
Member

@samuelcolvin samuelcolvin left a comment

Choose a reason for hiding this comment

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

LGTM, if you're happy please merge and check it all seems to be working correctly.

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