feat: default config #383

Venoox · 2023-06-08T17:44:48Z

Resolves #327

@rndquu I think even relative path is not gonna work because the @vercel/ncc bundler doesn't put it in the build process.

netlify · 2023-06-08T17:44:52Z

✅ Deploy Preview for ubiquibot-staging ready!

Name	Link
🔨 Latest commit	`cb5c231`
🔍 Latest deploy log	https://app.netlify.com/sites/ubiquibot-staging/deploys/64c762e83b03290008c821aa
😎 Deploy Preview	https://deploy-preview-383--ubiquibot-staging.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

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

rndquu · 2023-06-08T20:00:51Z

@rndquu I think even relative path is not gonna work because the @vercel/ncc bundler doesn't put it in the build process.

Then we should try using __dirname or smth else according to the docs

Venoox · 2023-06-09T05:46:47Z

@rndquu I think even relative path is not gonna work because the @vercel/ncc bundler doesn't put it in the build process.

This we should try using __dirname or smth else according to the docs

I tried locally with __dirname and when I run yarn build:serverless, the file is not copied over to the dist folder

rndquu · 2023-06-09T08:02:16Z

@rndquu I think even relative path is not gonna work because the @vercel/ncc bundler doesn't put it in the build process.

This we should try using __dirname or smth else according to the docs

I tried locally with __dirname and when I run yarn build:serverless, the file is not copied over to the dist folder

I've just tried with the following code and the config was copied to the dist folder:

const DEFAULT_CONFIG_FILE = `${__dirname}/../../ubiquibot-config-default.yml`;

Could you:

refactor code using __dirname
run yarn build:serverless
run node dist/index.js (there should be no errors) + ubiquibot-config-default.yml should be copied to the dist folder
if everything is fine then update the PR

0xcodercrane · 2023-06-09T08:53:29Z

@Venoox please make the PR a draft if its not ready for a reivew

Venoox · 2023-06-09T20:35:37Z

Now the file gets copied over but there is another issue:
I'm guessing it has something to do with the way ncc orders the code?

const parsedDefaultConfig = (0, private_1.parseYAML)(defaultConfig);
                                                    ^

TypeError: (0 , private_1.parseYAML) is not a function
    at 59612 (/Users/tomaz/ubiquibot/dist/index.js:149514:53)
    at __nccwpck_require__ (/Users/tomaz/ubiquibot/dist/index.js:208441:43)
    at 55447 (/Users/tomaz/ubiquibot/dist/index.js:149678:19)
    at __nccwpck_require__ (/Users/tomaz/ubiquibot/dist/index.js:208441:43)
    at 80417 (/Users/tomaz/ubiquibot/dist/index.js:147390:19)
    at __nccwpck_require__ (/Users/tomaz/ubiquibot/dist/index.js:208441:43)
    at 92040 (/Users/tomaz/ubiquibot/dist/index.js:147353:23)
    at __nccwpck_require__ (/Users/tomaz/ubiquibot/dist/index.js:208441:43)
    at 39321 (/Users/tomaz/ubiquibot/dist/index.js:147182:16)
    at __nccwpck_require__ (/Users/tomaz/ubiquibot/dist/index.js:208441:43)

Node.js v20.2.0

0x4007 · 2023-06-10T05:43:08Z

const parsedDefaultConfig = (0, private_1.parseYAML)(defaultConfig);

Yeah that definitely doesn't look like a function! Perhaps they already have an open issue on this.

You might also be able to experiment with pulling out the function's contents and running them without calling the function. I'm not sure.

rndquu · 2023-06-10T22:00:21Z

Now the file gets copied over but there is another issue: I'm guessing it has something to do with the way ncc orders the code?

const parsedDefaultConfig = (0, private_1.parseYAML)(defaultConfig);
                                                    ^

TypeError: (0 , private_1.parseYAML) is not a function
    at 59612 (/Users/tomaz/ubiquibot/dist/index.js:149514:53)
    at __nccwpck_require__ (/Users/tomaz/ubiquibot/dist/index.js:208441:43)
    at 55447 (/Users/tomaz/ubiquibot/dist/index.js:149678:19)
    at __nccwpck_require__ (/Users/tomaz/ubiquibot/dist/index.js:208441:43)
    at 80417 (/Users/tomaz/ubiquibot/dist/index.js:147390:19)
    at __nccwpck_require__ (/Users/tomaz/ubiquibot/dist/index.js:208441:43)
    at 92040 (/Users/tomaz/ubiquibot/dist/index.js:147353:23)
    at __nccwpck_require__ (/Users/tomaz/ubiquibot/dist/index.js:208441:43)
    at 39321 (/Users/tomaz/ubiquibot/dist/index.js:147182:16)
    at __nccwpck_require__ (/Users/tomaz/ubiquibot/dist/index.js:208441:43)

Node.js v20.2.0

Which command throws this error? This one node dist/index.js ?

Venoox · 2023-06-13T18:08:34Z

Which command throws this error? This one node dist/index.js ?

Yes. It seems it's an issue with top level code. If I wrap the code in a function then the error is gone.

Now there are two solutions, either we read the default config in getWideConfig and pass defaults as the last parameter in the function like getChainId(parsedRepo, parsedOrg, default['chain-id']), or load the config in the functions themselves using a singleton variable to prevent loading multiple times

rndquu · 2023-06-13T21:21:44Z

Which command throws this error? This one node dist/index.js ?

Yes. It seems it's an issue with top level code. If I wrap the code in a function then the error is gone.

Now there are two solutions, either we read the default config in getWideConfig and pass defaults as the last parameter in the function like getChainId(parsedRepo, parsedOrg, default['chain-id']), or load the config in the functions themselves using a singleton variable to prevent loading multiple times

I don't get any errors on node dist/index.js in this PR's branch though I use a slightly different node version v18.14.0

Perhaps you should leave this PR in its current state and open a QA issue so that we could see how it works in your repo?

Venoox · 2023-06-14T20:08:39Z

https://github.com/Venoox/ubiquibot/actions/runs/5271555816/jobs/9532609741

As I understand this error: private.ts imports helpers.ts which executes parseYAML but parseYAML is defined later in private.ts, so it's basically a dependency cycle?

rndquu · 2023-06-14T21:31:27Z

https://github.com/Venoox/ubiquibot/actions/runs/5271555816/jobs/9532609741

As I understand this error: private.ts imports helpers.ts which executes parseYAML but parseYAML is defined later in private.ts, so it's basically a dependency cycle?

yes, seems like that

perhaps here you could refactor to:

const parsedDefaultConfig: WideRepoConfig = YAML.parse(defaultConfig);

The YAML.parse() does not return a promise so it's ok to use it at the top of the file

Venoox · 2023-06-15T07:32:19Z

Seems like the issue is solved but there is an unrelated error regarding telegram bot (https://github.com/Venoox/ubiquibot/actions/runs/5275990184/jobs/9542118341)

rndquu · 2023-06-15T08:11:27Z

Seems like the issue is solved but there is an unrelated error regarding telegram bot (https://github.com/Venoox/ubiquibot/actions/runs/5275990184/jobs/9542118341)

It's missing here https://github.com/Venoox/ubiquibot/blob/default-config/.github/workflows/bot.yml but I think it's ok to pass it empty for QA workflow runs not related to Telegram issues

rndquu · 2023-06-15T08:13:36Z

@0xcodercrane could you review this PR again?

@Venoox thank you for fixing errors

0xcodercrane · 2023-06-15T13:15:49Z

@0xcodercrane could you review this PR again?

@Venoox thank you for fixing errors

Sure!

0xcodercrane · 2023-06-15T14:02:14Z

In terms of code review, I had a couple of changes for @Venoox but committed what I want to the Venoox:default-config to save hours. To be honest, I didn't get a chance to try to build so please take my changes into consideration as suggested changes and fix build issues if needed.
Once the build issue fixed, we can go to the QA step

rndquu · 2023-06-15T15:32:52Z

@Venoox could you fix typescript errors? Notice there are some changes introduced by @0xcodercrane

rndquu · 2023-06-16T21:10:23Z

@Venoox thank you

@0xcodercrane pls review again

0x4007 · 2023-07-27T19:21:16Z

Hey @Venoox really sorry about the delay I decided to manually try and take this over but I'm having issues resolving those merge conflicts.

My progress is here:

https://github.com/Venoox/ubiquibot/pull/18#pullrequestreview-1550716443

0xcodercrane · 2023-07-31T01:41:01Z

To wrap up the issue asap.

We should resolve conflicts first.
Regenerate the QA because I am seeing more comments after the last QA and review. @Draeieg
Any configuration change PRs need to be created on staging/production @rndquu

If you @Venoox can resolve the conflicts, we will appreciate it a lot.

Venoox · 2023-07-31T07:34:23Z

@0xcodercrane I resolved the conflicts

0xcodercrane · 2023-07-31T07:36:22Z

@0xcodercrane I resolved the conflicts

Thank you. To merge the PR, I will wait for the rest 2 items. @Venoox would you re-post the QA link with the latest codebase?

0x4007 · 2023-07-31T08:39:24Z

Lol @0xcodercrane I think it would have made more sense to ask @Venoox to resolve the merge conflicts as the last step.

Alternatively we can pause merging into development and rush merging this in so that they don't have to keep doing more unnecessary work. But due to all the merge delays lately I'm not a fan of intentionally adding to the delays.

0xcodercrane · 2023-07-31T12:42:50Z

Lol @0xcodercrane I think it would have made more sense to ask @Venoox to resolve the merge conflicts as the last step.

Alternatively we can pause merging into development and rush merging this in so that they don't have to keep doing more unnecessary work. But due to all the merge delays lately I'm not a fan of intentionally adding to the delays.

Ok. we can skip the QA processing for now. but need to wait @rndquu 's input for configuration changes

rndquu · 2023-07-31T13:54:02Z

Lol @0xcodercrane I think it would have made more sense to ask @Venoox to resolve the merge conflicts as the last step.
Alternatively we can pause merging into development and rush merging this in so that they don't have to keep doing more unnecessary work. But due to all the merge delays lately I'm not a fan of intentionally adding to the delays.

Ok. we can skip the QA processing for now. but need to wait @rndquu 's input for configuration changes

The PR looks good, no need for additional config update PRs

feat: default config

ffa2cb6

Venoox requested a review from 0xcodercrane as a code owner June 8, 2023 17:44

Venoox marked this pull request as draft June 9, 2023 20:27

feat: add __dirname for serverless build

add0b37

Merge branch 'ubiquity:development' into default-config

8091e66

fix: use YAML.parse directly because of depedency cycle

bba3336

rndquu marked this pull request as ready for review June 15, 2023 08:12

0xcodercrane added 2 commits June 15, 2023 21:55

feat: create a default config method

5c03ceb

feat: add default args

ce1a237

fix: typescript errors

653fa8d

0xcodercrane previously approved these changes Jun 17, 2023

View reviewed changes

rndquu mentioned this pull request Jul 14, 2023

All capabilities off by default #505

Closed

Draeieg mentioned this pull request Jul 14, 2023

QA for default config ubiquibot/staging#60

Open

rndquu mentioned this pull request Jul 19, 2023

build: workflow to automate the PR creation #540

Closed

0x4007 enabled auto-merge July 27, 2023 19:07

rndquu mentioned this pull request Jul 30, 2023

Add sync workflow for config param names #357

Closed

Merge remote-tracking branch 'upstream/development' into default-config

cb5c231

auto-merge was automatically disabled July 31, 2023 07:29
Head branch was pushed to by a user without write access

Venoox dismissed 0x4007’s stale review via cb5c231 July 31, 2023 07:29

0x4007 requested review from Draeieg and removed request for Draeieg July 31, 2023 08:39

Draeieg mentioned this pull request Jul 31, 2023

QA for feat: default config ubiquibot/staging#84

Open

rndquu approved these changes Jul 31, 2023

View reviewed changes

0xcodercrane added this pull request to the merge queue Jul 31, 2023

Merged via the queue into ubiquity:development with commit d85196e Jul 31, 2023

0xcodercrane mentioned this pull request Aug 2, 2023

Hotfix of default config PR #572

Merged

This was referenced Aug 17, 2023

fix: derive weight automatically #627

Merged

feat: extract duration dynamically from time label name #638

Closed

0x4007 mentioned this pull request Aug 26, 2023

Conversation Rewards Hot Fixes #673

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: default config #383

feat: default config #383

Venoox commented Jun 8, 2023 •

edited by rndquu

Loading

netlify bot commented Jun 8, 2023 •

edited

Loading

rndquu commented Jun 8, 2023 •

edited

Loading

Venoox commented Jun 9, 2023

rndquu commented Jun 9, 2023

0xcodercrane commented Jun 9, 2023

Venoox commented Jun 9, 2023 •

edited

Loading

0x4007 commented Jun 10, 2023

rndquu commented Jun 10, 2023

Venoox commented Jun 13, 2023

rndquu commented Jun 13, 2023

Venoox commented Jun 14, 2023

rndquu commented Jun 14, 2023

Venoox commented Jun 15, 2023

rndquu commented Jun 15, 2023 •

edited

Loading

rndquu commented Jun 15, 2023

0xcodercrane commented Jun 15, 2023

0xcodercrane commented Jun 15, 2023 •

edited

Loading

rndquu commented Jun 15, 2023

rndquu commented Jun 16, 2023

0x4007 commented Jul 27, 2023 •

edited

Loading

0xcodercrane commented Jul 31, 2023 •

edited

Loading

Venoox commented Jul 31, 2023

0xcodercrane commented Jul 31, 2023

0x4007 commented Jul 31, 2023 •

edited

Loading

0xcodercrane commented Jul 31, 2023

rndquu commented Jul 31, 2023

feat: default config #383

feat: default config #383

Conversation

Venoox commented Jun 8, 2023 • edited by rndquu Loading

netlify bot commented Jun 8, 2023 • edited Loading

✅ Deploy Preview for ubiquibot-staging ready!

rndquu commented Jun 8, 2023 • edited Loading

Venoox commented Jun 9, 2023

rndquu commented Jun 9, 2023

0xcodercrane commented Jun 9, 2023

Venoox commented Jun 9, 2023 • edited Loading

0x4007 commented Jun 10, 2023

rndquu commented Jun 10, 2023

Venoox commented Jun 13, 2023

rndquu commented Jun 13, 2023

Venoox commented Jun 14, 2023

rndquu commented Jun 14, 2023

Venoox commented Jun 15, 2023

rndquu commented Jun 15, 2023 • edited Loading

rndquu commented Jun 15, 2023

0xcodercrane commented Jun 15, 2023

0xcodercrane commented Jun 15, 2023 • edited Loading

rndquu commented Jun 15, 2023

rndquu commented Jun 16, 2023

0x4007 commented Jul 27, 2023 • edited Loading

0xcodercrane commented Jul 31, 2023 • edited Loading

Venoox commented Jul 31, 2023

0xcodercrane commented Jul 31, 2023

0x4007 commented Jul 31, 2023 • edited Loading

0xcodercrane commented Jul 31, 2023

rndquu commented Jul 31, 2023

Venoox commented Jun 8, 2023 •

edited by rndquu

Loading

netlify bot commented Jun 8, 2023 •

edited

Loading

rndquu commented Jun 8, 2023 •

edited

Loading

Venoox commented Jun 9, 2023 •

edited

Loading

rndquu commented Jun 15, 2023 •

edited

Loading

0xcodercrane commented Jun 15, 2023 •

edited

Loading

0x4007 commented Jul 27, 2023 •

edited

Loading

0xcodercrane commented Jul 31, 2023 •

edited

Loading

0x4007 commented Jul 31, 2023 •

edited

Loading