Skip to content
This repository has been archived by the owner on Sep 19, 2024. It is now read-only.

Default labels #506

Merged
merged 6 commits into from
Jul 25, 2023
Merged

Default labels #506

merged 6 commits into from
Jul 25, 2023

Conversation

whilefoo
Copy link
Collaborator

@whilefoo whilefoo commented Jul 13, 2023

Resolves #500

QA issue: whilefoo#4

@netlify
Copy link

netlify bot commented Jul 13, 2023

Deploy Preview for ubiquibot-staging ready!

Name Link
🔨 Latest commit 9e4ae94
🔍 Latest deploy log https://app.netlify.com/sites/ubiquibot-staging/deploys/64b5a4b925c498000862912c
😎 Deploy Preview https://deploy-preview-506--ubiquibot-staging.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@whilefoo whilefoo changed the title feat: default labels config Default labels Jul 13, 2023
@whilefoo whilefoo marked this pull request as ready for review July 13, 2023 21:04
Comment on lines 36 to 39
users:
pavlovcik:
- "Time: <1 Hour"
- "Priority: 0 (Normal)"
Copy link
Member

@0x4007 0x4007 Jul 13, 2023

Choose a reason for hiding this comment

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

Pretty cool you implemented this but I still fear that we'll need to remove this when we implement the database style config so that we don't confuse future partners when they have conflicting settings e.g.

/default-labels @pavlovcik "Time: <1 Hour" "Priority: 0 (Normal)"
  users:
    pavlovcik:
      - "Time: <1 Day"
      - "Priority: 1 (Medium)"

Copy link
Member

@0x4007 0x4007 left a comment

Choose a reason for hiding this comment

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

Code seems fine I just fear that we need to remove the user specific default label config code soon.

@@ -47,6 +47,40 @@ To test the bot, you can:
2. Add a time label, ex: `Time: <1 Day`
3. At this point the bot should add a price label.

## Configuration
Copy link
Member

Choose a reason for hiding this comment

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

You did a great job here thanks.

0xcodercrane
0xcodercrane previously approved these changes Jul 14, 2023
- "Priority: 0 (Normal)"
- "Test"
users:
pavlovcik:
Copy link
Contributor

@0xcodercrane 0xcodercrane Jul 14, 2023

Choose a reason for hiding this comment

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

oh no. seems better to use @ubiquity-bounties instead of @pavlovcik lol

Comment on lines 31 to 39
default-labels:
global:
- "Time: <1 Hour"
- "Priority: 0 (Normal)"
- "Test"
users:
pavlovcik:
- "Time: <1 Hour"
- "Priority: 0 (Normal)"
Copy link
Member

@0x4007 0x4007 Jul 16, 2023

Choose a reason for hiding this comment

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

There's one thing I forgot to highlight @whilefoo but #500 (comment)

Would you mind refactoring this config to be like this:

Suggested change
default-labels:
global:
- "Time: <1 Hour"
- "Priority: 0 (Normal)"
- "Test"
users:
pavlovcik:
- "Time: <1 Hour"
- "Priority: 0 (Normal)"
default-labels:
- "Time: <1 Hour"
- "Priority: 0 (Normal)"
- "Test"

I was on the fence about letting this slide but as I think about it more I'm just concerned that it'll cause conflicting config issues when we implement the slash command.

0x4007
0x4007 previously approved these changes Jul 16, 2023
Draeieg
Draeieg previously approved these changes Jul 17, 2023
@whilefoo whilefoo dismissed stale reviews from Draeieg and 0x4007 via 9e4ae94 July 17, 2023 20:29
@0x4007 0x4007 mentioned this pull request Jul 19, 2023
@whilefoo
Copy link
Collaborator Author

what's stopping this from merging?

@0x4007
Copy link
Member

0x4007 commented Jul 25, 2023

what's stopping this from merging?

That is unusual. GitHub iOS client UI doesn't tell me any useful information on it. @0xcodercrane do you know why

@0x4007 0x4007 merged commit cef8d3d into ubiquity:development Jul 25, 2023
@0xcodercrane
Copy link
Contributor

That is unusual. GitHub iOS client UI doesn't tell me any useful information on it. @0xcodercrane do you know why

Looks like related to unresolved conversations

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Auto Labeling Configuration
4 participants