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

Disable Auto Price Creation #158

Closed
0x4007 opened this issue Mar 3, 2023 · 20 comments · Fixed by #169
Closed

Disable Auto Price Creation #158

0x4007 opened this issue Mar 3, 2023 · 20 comments · Fixed by #169

Comments

@0x4007
Copy link
Member

0x4007 commented Mar 3, 2023

I noticed that every time I invoke the bot it repopulates all of the price labels. Please disable this behavior.

@0xcodercrane
Copy link
Contributor

0xcodercrane commented Mar 6, 2023

/start

@ubiquibot
Copy link

ubiquibot bot commented Mar 6, 2023

@0xcodercrane The time limit for this bounty is on 3/6/2023

@0xcodercrane 0xcodercrane linked a pull request Mar 6, 2023 that will close this issue
@0x4007
Copy link
Member Author

0x4007 commented Mar 6, 2023

I'll test this soon!

@ubiquibot
Copy link

ubiquibot bot commented Mar 6, 2023

@0xcodercrane ** [ CLAIM 50 ]** for 0x00868BB3BA2B36316c2fc42E4aFB6D4246b77E46

@0x4007
Copy link
Member Author

0x4007 commented Mar 6, 2023

@0xcodercrane ** [ CLAIM 50 ]** for 0x00868BB3BA2B36316c2fc42E4aFB6D4246b77E46

Hope you fix this appearance soon!

I just realized a small issue with this system.

For CI related tasks (technically I consider this bot CI) it's really difficult to test without merging. Any ideas on how to address this issue?

@rndquu

@rndquu
Copy link
Member

rndquu commented Mar 6, 2023

@0xcodercrane ** [ CLAIM 50 ]** for 0x00868BB3BA2B36316c2fc42E4aFB6D4246b77E46

Hope you fix this appearance soon!

I just realized a small issue with this system.

For CI related tasks (technically I consider this bot CI) it's really difficult to test without merging. Any ideas on how to address this issue?

@rndquu

Ideally you shouldn't test anything, it should be a bounty hunter's task to prove the issue works as expected.

A bounty hunter could:

  1. Create a PR from a branch of the forked repo, not a branch from the target repo
  2. Show how the feature works on the forked repo

@0x4007
Copy link
Member Author

0x4007 commented Mar 7, 2023

  1. I believe that you and Steve have done that already on the dollar repo and it still is an issue for me to verify that things work as expected with the CI sometimes. A great recent example: Setup Railway Deployment ubiquity-dollar#574

If you read that conversation history, you can understand the issue that I face: which is that it would be "easiest" to merge in and then test the CI in most cases. I think the simplest answer is that I have a disable auto payout command a maintainer can invoke for these situations.

For example /payout disable or something like that. Also we can have /payout enable

@rndquu
Copy link
Member

rndquu commented Mar 9, 2023

  1. I believe that you and Steve have done that already on the dollar repo and it still is an issue for me to verify that things work as expected with the CI sometimes. A great recent example: Setup Railway Deployment ubiquity-dollar#574

If you read that conversation history, you can understand the issue that I face: which is that it would be "easiest" to merge in and then test the CI in most cases. I think the simplest answer is that I have a disable auto payout command a maintainer can invoke for these situations.

For example /payout disable or something like that. Also we can have /payout enable

These commands /payout enable|disable add access control to the bounty bot, which adds complexity.

Can't we have a special branch like development-ci which is synced with the development branch? So PRs connected with the CI should be opened to the development-ci branch, then tested, the reopened to the developlment branch.

@0x4007
Copy link
Member Author

0x4007 commented Mar 9, 2023

access control to the bounty bot

Access control is already supported because it checks if you are apart of the organization or not. @0xcodercrane correct me if I am wrong

development-ci

Interesting idea

@rndquu
Copy link
Member

rndquu commented Mar 9, 2023

So we can create a task like Autosync development-ci branch with the development branch (via github action) + update docs for bounty hunters so that they know that PRs connected with the CI should be opened to the development-ci branch first.

@0x4007
Copy link
Member Author

0x4007 commented Mar 9, 2023

Sure let's try it out. Would you mind creating that bounty?

@rndquu
Copy link
Member

rndquu commented Mar 9, 2023

Sure let's try it out. Would you mind creating that bounty?

pls check ubiquity/ubiquity-dollar#603

@0x4007 0x4007 reopened this Apr 3, 2023
@0x4007
Copy link
Member Author

0x4007 commented Apr 3, 2023

Hey @0xcodercrane I just set up the bot on ubiquity/pay.ubq.fi#15 (awesome to see how simple it is now, just need to enable the repository access for the bot, and then update a label!)

Unfortunately it seems that you forgot the range labels:

image

@ubiquibot
Copy link

ubiquibot bot commented Apr 3, 2023

Releasing the bounty back to dev pool because the allocated duration already ended!

@ubiquibot
Copy link

ubiquibot bot commented Apr 3, 2023

@0xcodercrane The time limit for this bounty is on 4/4/2023

@ubiquibot
Copy link

ubiquibot bot commented Apr 8, 2023

Do you have any updates @0xcodercrane? If you would like to release the bounty back to the DevPool, please comment /unassign

@0xcodercrane
Copy link
Contributor

I will push soon

@0xcodercrane
Copy link
Contributor

13274fe

should be fixed with above ^^

@ubiquibot
Copy link

ubiquibot bot commented Apr 11, 2023

[ CLAIM 50 DAI ]

0x00868BB...6b77E46

@ubiquibot ubiquibot bot added the Paid label Apr 11, 2023
@0x4007
Copy link
Member Author

0x4007 commented Apr 11, 2023

@Steveantor this Paid label is a feature that I'm not aware of. I disagree with this approach because @rndquu already implemented cross referencing permits with etherescan transactions to actually verify that the payment has been made. I presume that you are simply adding the label "Paid" when it writes the permit which is misleading, because it doesn't actually check that the payment was made.

Please remove this feature.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants