Removed outdated section of primary package.json and add jsdoc comments by amoweolubusayo · Pull Request #2303 · ethereum-optimism/optimism

amoweolubusayo · 2022-03-09T23:06:40Z

Description
A clear and concise description of the features you're adding in this pull request.

Additional context
Add any other context about the problem you're solving.

Metadata

Fixes #[Link to Issue]

changeset-bot · 2022-03-09T23:06:43Z

🦋 Changeset detected

Latest commit: 19e1709

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

This PR includes changesets to release 4 packages

Name	Type
@eth-optimism/contracts	Patch
@eth-optimism/data-transport-layer	Patch
@eth-optimism/sdk	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

Add jsdoc comments to functions in contract package

tynes · 2022-03-10T01:59:56Z

packages/contracts/src/contract-defs.ts

 }

 export const getContractFactory = (
+  //returns the interface,bytecode and signer of a given contract by name.


These are non standard jsdoc comments, usually they go above the function definition with a particular format

Yep like @tynes said, here's an example of a standard jsdoc comment:

optimism/packages/core-utils/src/optimism/alias.ts

Lines 14 to 19 in 3525635

/**

* Applies the L1 => L2 aliasing scheme to an address.

*

* @param address Address to apply the scheme to.

* @returns Address with the scheme applied.

*/

Thank you so much! My bad. I would correct my PR.

New comments look great! Thank you :-)

Got the new comments..would work on those! Thanks!

Done. changeset added and rebase done.

Add jsdoc comments to contract-defs.ts

smartcontracts

Looking good, great job! Two quick things to resolve:

Let's add a changeset for the contracts package so that your new jsdoc comments are published as part of the @eth-optimism/contracts package. You can see instructions for how to do this here.
Let's also combine all of these changes into one commit that uses the conventional commit format. You can do this by running the command git rebase --interactive and following the instructions that appear there.

maurelian · 2022-03-11T15:04:31Z

packages/contracts/src/contract-defs.ts

-
-export const getContractDefinition = (name: string): any => {
+/** 
+ * Gets the contract's artifact


It's a nit, but I think this should have a period after it for consistency.

This is what was done in the example given in @smartcontracts review:

optimism/packages/core-utils/src/optimism/alias.ts

Lines 14 to 19 in 3525635

/**

* Applies the L1 => L2 aliasing scheme to an address.

*

* @param address Address to apply the scheme to.

* @returns Address with the scheme applied.

*/

Thank you for pointing that out.

There are merge conflicts.. Am I to resolve that?

Yes please.

To do that, I would:

fix the conflicts in package.json

rerun yarn, which will overwrite the yarn.lock file with the correct contents.

Also don't forget to combine the commits per the comment above. :)

Thanks! I have made the updates

Checks are failing. What can I do?

rename getContractDefinition to getContractArtifact

Fixed typescript linting error in contract-defs.ts

maurelian · 2022-03-17T13:27:47Z

@amoweolubusayo sorry but it looks like you'll have to pull from master again, and then please also combine all of these changes into one commit that uses the conventional commit format. You can do this by running the command git rebase --interactive and following the instructions that appear there.

… into develop

mergify · 2022-03-31T18:21:36Z

Hey @amoweolubusayo! This PR has merge conflicts. Please fix them before continuing review.

github-actions · 2022-04-19T02:12:08Z

This PR is stale because it has been open 14 days with no activity. Remove stale label or comment or this will be closed in 5 days.

… actors (#2303) ## Description This PR does some structural services to the `RollupNodeService` trait, in particular: - It ensures we're only passing static configurations/builders to the `NodeActor`s and offsets the building process to the `NodeActor` itself. - Each actor now has an associated builder. This unifies the terminology between `*Builder`s and `*Launcher`s which seemed to refer to the same concept. - This allows to simplify the `RollupNodeService` trait by having: - less constraints on the configurations accepted by the `NodeActors`. - having simpler associated methods which are basically just getters over some fields of the `RollupNode`. No need for async associated methods anymore or error handling. - The only `.await` we're using is to spin up the actors once they're initiated.

## Description This PR refactors the RPC actor in light of #2303 to unify its interactions with the other actors. Simplifies a little bit the way we handle RPC arguments by removing the `RpcBuilder` and move the `RpcModule` handling inside the `RpcActor`

Removed outdated section of primary package.json

c8c1fd5

amoweolubusayo mentioned this pull request Mar 9, 2022

Removed outdated section of primary package.json #2280

Closed

Add jsdoc comments

05103d0

Add jsdoc comments to functions in contract package

amoweolubusayo requested review from elenadimitrova, maurelian and smartcontracts as code owners March 9, 2022 23:18

github-actions bot added 2-reviewers labels Mar 9, 2022

amoweolubusayo changed the title ~~Removed outdated section of primary package.json~~ Removed outdated section of primary package.json and add jsdoc comments Mar 9, 2022

tynes reviewed Mar 10, 2022

View reviewed changes

Add jsdoc comments

98c7215

Add jsdoc comments to contract-defs.ts

smartcontracts reviewed Mar 10, 2022

View reviewed changes

-- changeset added

fc22e69

maurelian reviewed Mar 11, 2022

View reviewed changes

amoweolubusayo added 7 commits March 11, 2022 16:12

Update contract-defs.ts

7161351

feat: add changeset and resolve merge conflicts

8a5bf70

rename getContractDefinition in contract-defs.ts

5bc4bba

rename getContractDefinition to getContractArtifact

fix: typescript linting error

64f9513

Fixed typescript linting error in contract-defs.ts

fix: global variable getContractDefinition

44fb405

fix: yarn lock merge conflict

eb66747

fix: merge conflicts in yarn.lock

023b1b6

amoweolubusayo requested review from cfromknecht and mslipper as code owners March 16, 2022 17:38

Merge branch 'develop' of https://github.com/ethereum-optimism/optimism…

19e1709

… into develop

github-actions bot added M-ci Meta: ci related work M-dtl A-pkg-sdk Area: packages/sdk labels Mar 18, 2022

mergify bot requested review from tuxcanfly and removed request for elenadimitrova April 4, 2022 18:45

mergify bot added the conflict label Apr 4, 2022

github-actions bot added the Stale label Apr 19, 2022

github-actions bot closed this Apr 25, 2022

	/**
	* Applies the L1 => L2 aliasing scheme to an address.
	*
	* @param address Address to apply the scheme to.
	* @returns Address with the scheme applied.
	*/

Conversation

amoweolubusayo commented Mar 9, 2022

Uh oh!

changeset-bot bot commented Mar 9, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

smartcontracts left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

maurelian commented Mar 17, 2022

Uh oh!

mergify bot commented Mar 31, 2022

Uh oh!

github-actions bot commented Apr 19, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

changeset-bot bot commented Mar 9, 2022 •

edited

Loading