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.

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

Enable source module imports in stepper #1453

Merged
merged 6 commits into from
Jul 31, 2023
Merged

Conversation

Ziwen510
Copy link
Contributor

@Ziwen510 Ziwen510 commented Jul 24, 2023

Description

This pull request integrates source module imports within the stepper. The module objects manifest in their REPL string representation in the stepper. Any generated functions in the middle of the process are displayed as [Function].

The test cases are commented out before mock module functions become available.

@Ziwen510 Ziwen510 requested a review from ianyong July 24, 2023 09:30
@Ziwen510 Ziwen510 self-assigned this Jul 24, 2023
@Ziwen510 Ziwen510 linked an issue Jul 24, 2023 that may be closed by this pull request
@coveralls
Copy link

coveralls commented Jul 24, 2023

Pull Request Test Coverage Report for Build 5710720150

  • 55 of 183 (30.05%) changed or added relevant lines in 5 files are covered.
  • 2 unchanged lines in 2 files lost coverage.
  • Overall coverage decreased (-0.7%) to 82.889%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/mocks/context.ts 1 3 33.33%
src/stepper/lib.ts 2 7 28.57%
src/stepper/util.ts 8 20 40.0%
src/stepper/converter.ts 6 35 17.14%
src/stepper/stepper.ts 38 118 32.2%
Files with Coverage Reduction New Missed Lines %
src/stepper/stepper.ts 1 64.46%
src/utils/astCreator.ts 1 82.18%
Totals Coverage Status
Change from base Build 5710515365: -0.7%
Covered Lines: 10736
Relevant Lines: 12526

💛 - Coveralls

Copy link
Member

@ianyong ianyong left a comment

Choose a reason for hiding this comment

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

This PR is missing a description and test cases, so I haven't fully reviewed it because I lack context of what is going on here.

I have some code quality comments from a quick look through, with the use of type assertions being one that might take a bit of effort to resolve.

.husky/pre-push Outdated Show resolved Hide resolved
src/utils/astCreator.ts Outdated Show resolved Hide resolved
src/stepper/stepper.ts Show resolved Hide resolved
@Ziwen510 Ziwen510 requested review from martin-henz and ianyong and removed request for ianyong July 29, 2023 16:26
@martin-henz
Copy link
Member

As discussed, let's handle the refactoring later, see #1457.

Copy link
Member

@martin-henz martin-henz left a comment

Choose a reason for hiding this comment

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

Looks good.

@martin-henz martin-henz merged commit 66ee980 into master Jul 31, 2023
1 check passed
@martin-henz martin-henz deleted the stepper-module-import branch July 31, 2023 06:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Stepper: Import statements cause errors
4 participants