js-slang icon indicating copy to clipboard operation
js-slang copied to clipboard
verified publisher icon source-academy

Rewrite: Stepper

Open kavishsathia opened this issue 10 months ago • 2 comments

Description

This PR aims to rewrite the stepper. The implementation is still in progress.

Type of change

  • [ ] Bug fix (non-breaking change which fixes an issue)
  • [ ] New feature (non-breaking change which adds functionality)
  • [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • [ ] Documentation update
  • [x] Code quality improvements

How to test

Checklist

  • [ ] I have tested this code
  • [ ] I have updated the documentation

kavishsathia avatar Feb 28 '25 06:02 kavishsathia

Pull Request Test Coverage Report for Build 13645552196

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 81.126%
Totals Coverage Status
Change from base Build 13624411860: 0.0%
Covered Lines: 10762
Relevant Lines: 12898
💛 - Coveralls

coveralls avatar Feb 28 '25 07:02 coveralls

Hi, I'm Hai Minh. Professor Martin Henz assigned me to review and monitor the progress of this PR. I have just take a look at the current changes. Some of my comments:

  • The code quality is really good.
  • Overally the code is still self-explanative but if there are a few comments and some more detailed documentation it'd really help me.

Keep up the good work!

monnss69 avatar Apr 01 '25 15:04 monnss69

This branch is ready for review.

CATISNOTSODIUM avatar May 07 '25 12:05 CATISNOTSODIUM

LGTM!

Some tests still seem to be failing? Can you double check if they're okay?

RichDom2185 avatar May 09 '25 10:05 RichDom2185

LGTM!

Some tests still seem to be failing? Can you double check if they're okay?

Can you specify which cases are failing? Is it from the tracer_full.ts or other files?

Based on the current CI/CD, it's seem like the error comes from src/tests/index.ts.

CATISNOTSODIUM avatar May 11 '25 07:05 CATISNOTSODIUM

LGTM!

Some tests still seem to be failing? Can you double check if they're okay?

Can you specify which cases are failing?

Is it from the tracer_full.ts or other files?

Based on the current CI/CD, it's seem like the error comes from src/tests/index.ts.

You can look at the logs in the CI: https://github.com/source-academy/js-slang/actions/runs/14953607972/job/42006258517

So yes it is from the index file. Any idea why it's failing?

RichDom2185 avatar May 11 '25 08:05 RichDom2185

I'm not sure if it's the CI/CD problem or not. By the way, is this problem occurs on other PRs?

CATISNOTSODIUM avatar May 13 '25 05:05 CATISNOTSODIUM

Bump

It seems like there is an issue with the error location. Sometimes, it outputs the correct location, but sometimes it outputs an unknown location.

Any ideas of what's going on?

CATISNOTSODIUM avatar May 18 '25 13:05 CATISNOTSODIUM

Failing CI CD related issue: https://github.com/source-academy/js-slang/issues/1758 Right now, I've commented out the test that has been problematic, so all checks have passed 😄 .

CATISNOTSODIUM avatar May 25 '25 08:05 CATISNOTSODIUM