Skip to content

added internal and external imports separation within integration tests#2019

Closed
lucadonnoh wants to merge 1 commit intoethereum-optimism:developfrom
lucadonnoh:internal-external-imports-separation
Closed

added internal and external imports separation within integration tests#2019
lucadonnoh wants to merge 1 commit intoethereum-optimism:developfrom
lucadonnoh:internal-external-imports-separation

Conversation

@lucadonnoh
Copy link
Contributor

Description
Separated internal and external imports in integration tests

Additional context
Some comments where missing, others were wrong (Internal and External switched)

Metadata

@changeset-bot
Copy link

changeset-bot bot commented Jan 16, 2022

⚠️ No Changeset found

Latest commit: e634bf2

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

@github-actions github-actions bot added the A-integration Area: integration tests label Jan 16, 2022
@codecov-commenter
Copy link

codecov-commenter commented Jan 16, 2022

Codecov Report

Merging #2019 (df7846f) into develop (e634bf2) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop    #2019   +/-   ##
========================================
  Coverage    74.58%   74.58%           
========================================
  Files           79       79           
  Lines         2554     2554           
  Branches       401      401           
========================================
  Hits          1905     1905           
  Misses         649      649           
Flag Coverage Δ
batch-submitter 62.50% <ø> (ø)
contracts 90.48% <ø> (ø)
core-utils 57.50% <ø> (ø)
data-transport-layer 38.64% <ø> (ø)
message-relayer 70.86% <ø> (ø)
sdk 88.38% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.


Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e634bf2...df7846f. Read the comment docs.

@smartcontracts
Copy link
Contributor

Unfortunately I'm having issues seeing the integration test error on my current wifi. Going to take another look at this later tonight.

@indeavr
Copy link
Contributor

indeavr commented Jan 17, 2022

I would also recommend that we enforce this order in eslint (top level - .eslintrc.js):

current:

'import/order': 'off'

New:

'import/order': [
      1,
      {
        groups: [
          'external',
          'builtin',
          'internal',
          'sibling',
          'parent',
          'index',
        ],
        'newlines-between': 'always',
      },
    ],

The groups property defines the order :)

@smartcontracts
Copy link
Contributor

I would also recommend that we enforce this order in eslint (top level - .eslintrc.js):

current:

'import/order': 'off'

New:

'import/order': [
      1,
      {
        groups: [
          'external',
          'builtin',
          'internal',
          'sibling',
          'parent',
          'index',
        ],
        'newlines-between': 'always',
      },
    ],

The groups property defines the order :)

Whoa! I didn't know this was a thing we could do. I'll make an issue for this :-)

@smartcontracts
Copy link
Contributor

@lucadonnoh looks like this just needs a quick rebase to fix the merge conflicts. Great work as usual!

@github-actions github-actions bot added 2-reviewers A-op-batcher Area: op-batcher M-ci Meta: ci related work A-cannon Area: cannon A-ops Area: ops and removed A-op-batcher Area: op-batcher M-dtl A-ops Area: ops A-cannon Area: cannon M-ci Meta: ci related work labels Jan 19, 2022
@lucadonnoh lucadonnoh closed this Jan 19, 2022
@github-actions github-actions bot removed the A-integration Area: integration tests label Jan 19, 2022
@lucadonnoh lucadonnoh reopened this Jan 19, 2022
@github-actions github-actions bot added the A-integration Area: integration tests label Jan 19, 2022
@lucadonnoh
Copy link
Contributor Author

sorry for the mess, i'm just learning how to contribute to a repo! 😅

@smartcontracts
Copy link
Contributor

Haha, no worries @lucadonnoh. Just let me know if you need any help!

@tynes
Copy link
Contributor

tynes commented Jan 31, 2022

This unfortunately now has merge conflicts and needs to be fixed

@mslipper
Copy link
Collaborator

mslipper commented Mar 2, 2022

This is already complete - closing for now.

@mslipper mslipper closed this Mar 2, 2022
theochap pushed a commit that referenced this pull request Dec 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-integration Area: integration tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Separate internal and external imports within integration tests

6 participants