Skip to content

feat(ctb): SystemConfig comments#3901

Merged
mergify[bot] merged 1 commit intodevelopfrom
sc/ctb-sys-config-comments
Nov 7, 2022
Merged

feat(ctb): SystemConfig comments#3901
mergify[bot] merged 1 commit intodevelopfrom
sc/ctb-sys-config-comments

Conversation

@smartcontracts
Copy link
Copy Markdown
Contributor

@smartcontracts smartcontracts commented Nov 7, 2022

Description
Adds comments to SystemConfig functions and variables.

@changeset-bot
Copy link
Copy Markdown

changeset-bot bot commented Nov 7, 2022 •
edited
Loading

🦋 Changeset detected

Latest commit: 427831d

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 5 packages
Name Type
@eth-optimism/contracts-bedrock Patch
@eth-optimism/actor-tests Patch
@eth-optimism/sdk Patch
@eth-optimism/drippie-mon Patch
@eth-optimism/message-relayer Patch

Not sure what this means? Click here to learn what changesets are.

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

@github-actions github-actions bot added 2-reviewers A-pkg-contracts-bedrock Area: packages/contracts-bedrock labels Nov 7, 2022
protolambda
protolambda approved these changes Nov 7, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@protolambda protolambda left a comment

Choose a reason for hiding this comment

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

LGTM, thank you for improving this, sorry about not commenting the contract earlier.

tynes
tynes reviewed Nov 7, 2022
View reviewed changes
tynes
tynes reviewed Nov 7, 2022
View reviewed changes
tynes
tynes reviewed Nov 7, 2022
View reviewed changes
tynes
tynes reviewed Nov 7, 2022
View reviewed changes
@tynes
Copy link
Copy Markdown
Contributor

tynes commented Nov 7, 2022

Generally looks good to me, could use a little more polish with some of the docstrings, but ok with merging as is. Left a few questions that I have been thinking about regarding tweaks to this contract

@smartcontracts
feat(ctb): SystemConfig comments
427831d 
Adds comments to SystemConfig functions and variables.
@smartcontracts smartcontracts force-pushed the sc/ctb-sys-config-comments branch from 13ca5fd to 427831d Compare November 7, 2022 19:12
@mergify
Copy link
Copy Markdown
Contributor

mergify bot commented Nov 7, 2022

This PR has been added to the merge queue, and will be merged soon.

mergify bot added a commit that referenced this pull request Nov 7, 2022
@mergify
Merge of #3901
7b0033b
@mergify mergify bot mentioned this pull request Nov 7, 2022
Closed
95 tasks
@mergify
Copy link
Copy Markdown
Contributor

mergify bot commented Nov 7, 2022

This PR is next in line to be merged, and will be merged as soon as checks pass.

@mergify
Copy link
Copy Markdown
Contributor

mergify bot commented Nov 7, 2022

Hey @smartcontracts, this pull request failed to merge and has been dequeued from the merge train. If you believe your PR failed in the merge train because of a flaky test, requeue it by commenting with @mergifyio requeue.
More details can be found on the Queue: Embarked in merge train check-run.

@mergify mergify bot removed the on-merge-train label Nov 7, 2022
@smartcontracts
Copy link
Copy Markdown
Contributor Author

smartcontracts commented Nov 7, 2022

@Mergifyio requeue

@mergify
Copy link
Copy Markdown
Contributor

mergify bot commented Nov 7, 2022

requeue

✅ The queue state of this pull request has been cleaned. It can be re-embarked automatically

@mergify
Copy link
Copy Markdown
Contributor

mergify bot commented Nov 7, 2022

This PR has been added to the merge queue, and will be merged soon.

@mergify mergify bot mentioned this pull request Nov 7, 2022
Closed
95 tasks
@mergify
Copy link
Copy Markdown
Contributor

mergify bot commented Nov 7, 2022

This PR is next in line to be merged, and will be merged as soon as checks pass.

mergify bot added a commit that referenced this pull request Nov 7, 2022
@mergify
Merge of #3901
3ce83db
@mergify
Copy link
Copy Markdown
Contributor

mergify bot commented Nov 7, 2022

Hey @smartcontracts, this pull request failed to merge and has been dequeued from the merge train. If you believe your PR failed in the merge train because of a flaky test, requeue it by commenting with @mergifyio requeue.
More details can be found on the Queue: Embarked in merge train check-run.

@mergify mergify bot removed the on-merge-train label Nov 7, 2022
@smartcontracts
Copy link
Copy Markdown
Contributor Author

smartcontracts commented Nov 7, 2022

@Mergifyio requeue

@mergify
Copy link
Copy Markdown
Contributor

mergify bot commented Nov 7, 2022

requeue

✅ The queue state of this pull request has been cleaned. It can be re-embarked automatically

@mergify
Copy link
Copy Markdown
Contributor

mergify bot commented Nov 7, 2022

This PR has been added to the merge queue, and will be merged soon.

mergify bot added a commit that referenced this pull request Nov 7, 2022
@mergify
Merge of #3901
660d1d8
@mergify mergify bot mentioned this pull request Nov 7, 2022
Closed
95 tasks
mergify bot added a commit that referenced this pull request Nov 7, 2022
@mergify
Merge of #3901
4dec76f
@mergify
Copy link
Copy Markdown
Contributor

mergify bot commented Nov 7, 2022

This PR is next in line to be merged, and will be merged as soon as checks pass.

@mergify mergify bot merged commit b2c4085 into develop Nov 7, 2022
@mergify mergify bot deleted the sc/ctb-sys-config-comments branch November 7, 2022 21:05
@mergify mergify bot removed the on-merge-train label Nov 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tynes tynes tynes approved these changes

+1 more reviewer

@protolambda protolambda protolambda approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

A-pkg-contracts-bedrock Area: packages/contracts-bedrock

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@smartcontracts @tynes @protolambda