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

Update @typescript-eslint/parser for compatibility with used typescript #3708

Merged

Conversation

wwahammy
Copy link
Contributor

@wwahammy wwahammy commented Jan 14, 2024

When I run npm run lint I received an warning saying @typescript-eslint/typescript-estree (a dependency of @typescript-eslint/parser) did not support the version of Typescript used in the project.

This updates @typescript-eslint/parser to a more modern version without touching eslint itself (since I don't know what the consequences are of that)

--

I rebased this on #3707 so I can prove that the lint process still runs. I'm happy to rearrange if preferred.

@wwahammy wwahammy force-pushed the update-typescript-eslint-parser branch from a055d95 to c1d6549 Compare January 14, 2024 15:16
@ocdtrekkie
Copy link
Collaborator

This is probably approaching a place I should check with @kentonv if he is okay with me merging things here for now or if we should move to a fork or maybe dev branch now. Merging docs and test infrastructure code falls generally within "I did not break the official build" area, but as we exit Actions code I want to check if he would like the master branch here to remain as-is.

There's pros and cons to transferring the repo or forking it (things like stars, watchers, issues, etc.) but I do not really want to explore that until we are approaching an actual release plan which needs to factor in the database migration.

@ocdtrekkie
Copy link
Collaborator

I think the most appealing strategy to me personally would be to create a dev branch in this repo and merge new changes that impact the production build into that branch (we might need to adjust Actions on master to also build_and_test from it), which can be sort of a beta build of the fork. Later, when we release/fork/etc. we could rename that branch (probably to 'main' to reflect current standards, and set it as the default branch for sandstorm-org.

@ocdtrekkie ocdtrekkie changed the base branch from master to dev January 15, 2024 08:28
@ocdtrekkie
Copy link
Collaborator

As a note, we currently expect the linter to find 773 warnings: https://github.com/sandstorm-io/sandstorm/blob/master/.github/workflows/build.yml#L74

Ideally, we should not allow it to increment upwards. However, considering the linter logs from last time the linter actually ran is likely lost to time, it may be okay to increment it. That will allow the lint step of the Action to pass.

I also edited this PR to target the dev branch, which is currently even with master.

@ocdtrekkie ocdtrekkie merged commit 7b62b71 into sandstorm-io:dev Feb 5, 2024
1 check failed
@ocdtrekkie
Copy link
Collaborator

@wwahammy Looks like this got us further, but tests didn't run because of the lint error, so now that I've forgiven the lint error we got test failures!

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.

2 participants