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

Revert "Sync ERC permits" #185

Closed
wants to merge 1 commit into from

Conversation

molecula451
Copy link
Member

Reverts #177

move this changes to respective repo (https://github.com/ubiquity/audit.ubq.fi)

@Keyrxng

@ubiquity-os-deployer
Copy link

@molecula451
Copy link
Member Author

@Keyrxng can you handle the move to audit? https://github.com/ubiquity/audit.ubq.fi

@Keyrxng
Copy link
Member

Keyrxng commented Mar 3, 2024

So port my changes over to the new repo and then remove each of the ported apps or leave that for another issue?

@molecula451
Copy link
Member Author

molecula451 commented Mar 3, 2024

So port my changes over to the new repo and then remove each of the ported apps or leave that for another issue?

just work accordingly, let's revert this, if there are changes that belong to this repo then make a new PR to this, check that now we have:

https://github.com/ubiquity/audit.ubq.fi
https://github.com/ubiquity/onboard.ubq.fi
https://github.com/ubiquity/keygen.ubq.fi

@Keyrxng
Copy link
Member

Keyrxng commented Mar 3, 2024

I'm confused as to why the entire rewards app has been ported into the onboarding and audit repos, the only imports from rewards into the JS is a few constants (three of which the rpc handler will export) but they can be copy/pasted for now. I know the css is sort of bundled together and that'll have to stay too

If there is another reason why it's there I won't touch it

@molecula451
Copy link
Member Author

molecula451 commented Mar 3, 2024

I'm confused as to why the entire rewards app has been ported into the onboarding and audit repos, the only imports from rewards into the JS is a few constants (three of which the rpc handler will export) but they can be copy/pasted for now. I know the css is sort of bundled together and that'll have to stay too

If there is another reason why it's there I won't touch it

the only reason it's because package dependencies and code use, there's no other, as the plan it's to actually refactor these apps as a standalone and not in this same repo

but my concern is, if you're branch pr was merged then it's good, but it has audit changes, what the value of the audit c hanges here if it have to be ported to audit repo, audit it's getting purged here

@Keyrxng
Copy link
Member

Keyrxng commented Mar 3, 2024

actually I'm better targeting the dev branch with my PR rather than your revert which has removed a lot

@molecula451
Copy link
Member Author

actually I'm better targeting the dev branch with my PR rather than your revert which has removed a lot

it was opened just in case, if you move the changes to audit to the actual audit repo, we can just purge here with any other PR etc

@molecula451 molecula451 closed this Mar 4, 2024
@molecula451 molecula451 mentioned this pull request Mar 4, 2024
@0x4007 0x4007 deleted the revert-177-sync-erc-permits branch October 8, 2024 19:18
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