Skip to content

Conversation

@leeyi45
Copy link
Contributor

@leeyi45 leeyi45 commented Oct 9, 2024

Instead of doing the big jump and removing everything at once, I think it might be better if I broke down #1707 into several different PRs.

Will resolve:

@coveralls
Copy link

coveralls commented Oct 9, 2024

Pull Request Test Coverage Report for Build 13576250348

Details

  • 22 of 32 (68.75%) changed or added relevant lines in 7 files are covered.
  • 7 unchanged lines in 4 files lost coverage.
  • Overall coverage decreased (-0.3%) to 81.255%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/parser/source/typed/typeParser.ts 12 22 54.55%
Files with Coverage Reduction New Missed Lines %
src/runner/sourceRunner.ts 1 88.24%
src/utils/rttc.ts 1 96.69%
src/parser/source/typed/typeParser.ts 2 47.84%
src/index.ts 3 71.12%
Totals Coverage Status
Change from base Build 13555505176: -0.3%
Covered Lines: 11154
Relevant Lines: 13352

💛 - Coveralls

@RichDom2185 RichDom2185 self-requested a review October 9, 2024 04:32
@RichDom2185
Copy link
Member

Let me know when it's ready for review, thanks!

@leeyi45
Copy link
Contributor Author

leeyi45 commented Oct 9, 2024

Let me know when it's ready for review, thanks!

It is ready for review I guess, if we don't mind the coverage drop

Copy link
Member

@RichDom2185 RichDom2185 left a comment

Choose a reason for hiding this comment

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

Just some minor nits :)

@s-kybound
Copy link
Member

Hi, please run the command yarn upgrade string-width strip-ansi to update these two packages before trying this again!

@leeyi45
Copy link
Contributor Author

leeyi45 commented Feb 19, 2025

Hi, please run the command yarn upgrade string-width strip-ansi to update these two packages before trying this again!

@s-kybound I've done as requested, but I've now got a bunch of failing tests that I'm not sure what to do about

Copy link
Member

@s-kybound s-kybound left a comment

Choose a reason for hiding this comment

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

LGTM

@s-kybound s-kybound merged commit a50f7dc into master Feb 27, 2025
3 of 4 checks passed
@s-kybound s-kybound deleted the remove-non-det branch February 27, 2025 22:01
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.

5 participants