Skip to content

Comments

contracts: script for making markdown file with all our contract addresses#456

Merged
gakonst merged 4 commits intomasterfrom
feat/regenesis-generate-markdown
Apr 15, 2021
Merged

contracts: script for making markdown file with all our contract addresses#456
gakonst merged 4 commits intomasterfrom
feat/regenesis-generate-markdown

Conversation

@gigamesh
Copy link
Contributor

@gigamesh gigamesh commented Apr 13, 2021 •
edited
Loading

Credit to Pacien Boisson @pakocrew

Please review carefully as this will be the source of truth for the near future

@changeset-bot
Copy link

changeset-bot bot commented Apr 13, 2021 •
edited
Loading

⚠️ No Changeset found

Latest commit: e32b665

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@gigamesh gigamesh force-pushed the feat/regenesis-generate-markdown branch 2 times, most recently from 121858a to 53146f5 Compare April 13, 2021 17:04
gigamesh
gigamesh commented Apr 13, 2021
View reviewed changes
packages/contracts/package.json Outdated
Copy link
Contributor Author

@gigamesh gigamesh Apr 13, 2021

Choose a reason for hiding this comment

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

I wasn't sure of the best place to run it but figured it should happen here

Copy link
Contributor

@snario snario Apr 13, 2021

Choose a reason for hiding this comment

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

Probably should be part of the deploy script.

Copy link
Contributor Author

@gigamesh gigamesh Apr 13, 2021

Choose a reason for hiding this comment

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

moved 👍

@snario snario requested a review from smartcontracts April 13, 2021 17:16
tynes
tynes reviewed Apr 13, 2021
View reviewed changes
@gigamesh gigamesh force-pushed the feat/regenesis-generate-markdown branch from 63daf96 to 52e0bde Compare April 13, 2021 21:10
snario
snario suggested changes Apr 14, 2021
View reviewed changes
Copy link
Contributor

@snario snario left a comment

Choose a reason for hiding this comment

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

Instead of a column named Open could we just hyperlink the address of the contract to make the table width smaller?

@tynes
Copy link
Contributor

tynes commented Apr 14, 2021

Instead of a column named Open could we just hyperlink the address of the contract to make the table width smaller?

I'd support this

@gigamesh gigamesh force-pushed the feat/regenesis-generate-markdown branch from b76ad12 to 71f9850 Compare April 14, 2021 17:31
smartcontracts
smartcontracts reviewed Apr 14, 2021
View reviewed changes
packages/contracts/scripts/generate-markdown.js
const chainId = Number(
fs.readFileSync(`./deployments/${deployment}/.chainId`)
);
const network = networks[chainId];
Copy link
Contributor

@smartcontracts smartcontracts Apr 14, 2021

Choose a reason for hiding this comment

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

Could we add some sort of per-network annotations that can be injected into the markdown? E.g., kovan-v1 is outdated and will no longer be in use. We'll want to stop showing those addresses to people once we stop using the contracts. We probably want to add some sort of text to that section to make it clear that those contracts are deprecated. Also probably want to add the option to hide a deployment completely.

Copy link
Contributor

@gakonst gakonst Apr 15, 2021

Choose a reason for hiding this comment

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

Let's do this in a separate PR, if you have a specific idea could you please open an issue for it?

@smartcontracts
Copy link
Contributor

smartcontracts commented Apr 14, 2021

Also requesting that we add commit credits for @pakocrew

@gakonst gakonst mentioned this pull request Apr 15, 2021
Closed
@gakonst gakonst force-pushed the feat/regenesis-generate-markdown branch from 71f9850 to e32b665 Compare April 15, 2021 14:38
@gakonst
Copy link
Contributor

gakonst commented Apr 15, 2021 •
edited
Loading

I rebased and git commit --amend --author="Pacien Boisson <pakokrew@gmail.com" --no-edit the first commit to credit @pakokrew

@gakonst gakonst merged commit 3c92bdb into master Apr 15, 2021
@gakonst gakonst deleted the feat/regenesis-generate-markdown branch April 15, 2021 14:40
InoMurko referenced this pull request in omgnetwork/optimism May 25, 2021
@gigamesh @pacwoodson
contracts: script for making markdown file with all our contract addr…
98b4f9d 
…esses (#456)

* contracts: adds generate-markdown script for making markdown file with all our contract addresses

* Moves generate-markdown to deploy script

* Adds Goerli chain Id

* Removes etherscan Open column

Co-authored-by: Pacien Boisson <pakokrew@gmail.com>
bap2pecs added a commit to babylonlabs-io/optimism that referenced this pull request Jul 31, 2024
@bap2pecs
chore: improve logs (ethereum-optimism#456)
9766a34
theochap pushed a commit that referenced this pull request Dec 10, 2025
@refcell @clabby
fix(client): Walkback Channel Timeout (#456)
952390d 
* fix: downgrade for release plz (#458)

* chore(bin): Remove `kt` (#461)

* chore(bin): Remove `kt`

* lint

* lint

* fix(client): walkback channel timeout

* update tests

* lint

---------

Co-authored-by: clabby <ben@clab.by>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tynes tynes tynes left review comments

@smartcontracts smartcontracts smartcontracts left review comments

+2 more reviewers

@gakonst gakonst gakonst left review comments

@snario snario snario requested changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@gigamesh @tynes @smartcontracts @gakonst @snario @pacwoodson