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

Always run when pull opened #5

Open
0x4007 opened this issue Jan 29, 2025 · 13 comments · May be fixed by #6
Open

Always run when pull opened #5

0x4007 opened this issue Jan 29, 2025 · 13 comments · May be fixed by #6

Comments

@0x4007
Copy link
Member

0x4007 commented Jan 29, 2025

Sometimes authors don't open their pulls as drafts and as I understand this plugin only runs when converting from draft to regular pull.

The idea is that errors should not be thrown unless an actual error occurred. Errors shouldn't make the plugin annoying or impose where it's not needed. An example of this is the previous issue I filed in this repo #4

This should run when a specification is linked. So this includes on link event and/or when the pull body is edited to include a closing keyword.

Also I suppose that we should run the precheck when the pull is opened as well, even if it isn't a draft.

Originally posted by @0x4007 in ubiquity-os-marketplace/command-ask#58 (comment)

Copy link

ubiquity-os-beta bot commented Jan 29, 2025

@0x4007
Copy link
Member Author

0x4007 commented Jan 31, 2025

@ishowvel are you interested to make this adjustment?

@ishowvel
Copy link
Contributor

So refactoring some errors to just be warnings so that issues don't get spammed with bot comments and running pull precheck on pulls no matter the state?

@ishowvel
Copy link
Contributor

/start

Copy link

! 

@gentlementlegen
Copy link
Member

gentlementlegen commented Jan 31, 2025

^ the run was okay, only the display I messed up (which I fixed in ubiquity-os-marketplace/command-start-stop#133)

Error was
�[33m ⚠ You must be a core team member, or an administrator to start this task�[0m

@ishowvel
Copy link
Contributor

/start

Copy link

ubiquity-os-beta bot commented Jan 31, 2025

! 

@gentlementlegen @whilefoo this metadata is messed up it says a function named "async" called it.

Aggregate error needs to be fixed

@ishowvel
Copy link
Contributor

ishowvel commented Feb 1, 2025

@0x4007 can I be assigned to this?

Copy link

ubiquity-os-beta bot commented Feb 1, 2025

Important

  • Be sure to link a pull-request before the first reminder to avoid disqualification.
  • Reminders will be sent every 1 day and 18 hours if there is no activity.
  • Assignees will be disqualified after 3 days and 12 hours of inactivity.

@0x4007
Copy link
Member Author

0x4007 commented Feb 1, 2025

All plugins should try and be invisible as much as possible in normal GitHub use so they don't annoy our partners. So errors should only really throw when it's clear that the users are trying to use the feature. Commands are a great example where if you're using GitHub normally you're not writing these slash commands.

But things like opening pull requests is a bit more tricky so we need to be very strategic of when we throw errors. But I think since opening pulls is very normal for normal GitHub use, maybe we shouldn't throw errors at all.

@gentlementlegen
Copy link
Member

The display was broken but the error was legitimate. The user could not be assigned to this task because it is reserved for collaborators and admins.

@ishowvel
Copy link
Contributor

ishowvel commented Feb 2, 2025

Changes are done, gonna push to pr with a qa by tomorrow

@ishowvel ishowvel linked a pull request Feb 4, 2025 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants