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.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Bump dependencies flagged by dependabot #12

Merged
merged 5 commits into from Dec 2, 2022
Merged

Bump dependencies flagged by dependabot #12

merged 5 commits into from Dec 2, 2022

Conversation

jpribyl
Copy link
Owner

@jpribyl jpribyl commented Dec 2, 2022

No description provided.

@jpribyl
Copy link
Owner Author

jpribyl commented Dec 2, 2022

@mendhak if you're around, had a chance to look at this one

This was referenced Dec 2, 2022
Closed
Closed
Closed
Closed
@mendhak
Copy link
Collaborator

mendhak commented Dec 2, 2022

Oh nice you got the tests passing too. Sorry I couldn't be much help there, the errors didn't make much sense to me.

@jpribyl jpribyl mentioned this pull request Dec 2, 2022
Closed
@jpribyl
Copy link
Owner Author

jpribyl commented Dec 2, 2022

Yeah, the test issue was funky- it was a weird case where the test seemed to be attempting to use a version of post.ts that hadn't been transformed with ttsc -- the simplest thing wound up being to just remove the "fake" typescript parts and replace them with checks that didn't have to be transformed. Honestly, I had never heard of ttsc before encountering this project. It's a neat idea but seems to add a good deal of complexity

@jpribyl
Copy link
Owner Author

jpribyl commented Dec 2, 2022

The one thing I did remove without replacing was the check for whether the Manifest is actually a Manifest -- since we updated typescript to 4.9, I'm going to see if we can make use of the satisfies operator for this (see here for info)

mendhak
mendhak reviewed Dec 2, 2022
View reviewed changes
src/Tar.ts
assertManifests(manifests)
return manifests
const raw = await loadRawManifests(path);
const manifests = JSON.parse(raw.toString()) satisfies Manifests;
Copy link
Collaborator

Choose a reason for hiding this comment

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

I see, and looks like the builds are happy with it too. This work is satisfactory 馃槅

Copy link
Owner Author

Choose a reason for hiding this comment

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

awesome, I'll go ahead and merge it in- lmk if any issues come up- I'm not actively using this project a ton at the moment so you would likely find them before me 馃挭

@jpribyl jpribyl merged commit 4d9355e into main Dec 2, 2022
@jpribyl jpribyl deleted the bump-deps branch December 2, 2022 21:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mendhak mendhak mendhak approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@jpribyl @mendhak