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.

Sign up for GitHub

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

Jump to bottom

Add missing LINK token addresses #16023

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from
Open

Add missing LINK token addresses #16023

wants to merge 3 commits into from

Conversation

friedemannf
Copy link
Member

  • Add missing LINK token addresses
  • Rename zkSync to ZKsync

@friedemannf friedemannf requested review from a team as code owners January 21, 2025 23:35
@friedemannf friedemannf requested a review from a team January 22, 2025 00:04
jmank88
jmank88 previously approved these changes Jan 22, 2025
View reviewed changes
@cl-sonarqube-production
Copy link

Quality Gate passed Quality Gate passed

Issues
0 New issues
0 Fixed issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarQube

@friedemannf friedemannf requested a review from jmank88 January 23, 2025 12:39
jmank88
jmank88 reviewed Jan 23, 2025
View reviewed changes
core/chains/evm/config/toml/defaults/Zircuit_Sepolia.toml
Copy link
Contributor

Choose a reason for hiding this comment

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

These fields were alpha sorted before. Why move this field to the top?

Copy link
Member Author

Choose a reason for hiding this comment

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

I thought it would make sense to sort them by immutable->mutable and have the fields that we're not going to ever update at the top, like ChainID & LINK address

Copy link
Contributor

Choose a reason for hiding this comment

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

I could see either making sense but alpha sorting might be easier for everyone to align on. Tbh I didn't realize these were alpha sorted so maybe we need some way to enforce it. I noticed even fallback.toml was probably alpha sorted at some point but addition of new configs has thrown that off too. Either new configs were just added to the bottom or grouped with other related configs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jmank88 jmank88 jmank88 left review comments

@amit-momin amit-momin amit-momin approved these changes

@simsonraj simsonraj simsonraj approved these changes

@george-dorin george-dorin Awaiting requested review from george-dorin george-dorin is a code owner automatically assigned from smartcontractkit/foundations

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@friedemannf @jmank88 @simsonraj @amit-momin