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

fix: remove yarn.lock and restore rc files #122

Merged
merged 6 commits into from
May 10, 2024

Conversation

SilasPeters
Copy link
Member

@SilasPeters SilasPeters commented Apr 30, 2024

We currently use two packagers: npm and yarn. This is bad, and I thus propose to switch to npm.

Furthermore, in the vite PR we removed some .*rc files, which does not seem nessescary as far as I understand, and it would be better to keep them.

All linters seem to work, but see the other comments on why I am unsure to merge this.

@SilasPeters SilasPeters added maintainability dependencies Pull requests that update a dependency file labels Apr 30, 2024
@SilasPeters SilasPeters requested a review from Riscky April 30, 2024 21:23
@SilasPeters SilasPeters self-assigned this Apr 30, 2024
@SilasPeters SilasPeters force-pushed the chore/cleanup-config-files branch from 0411773 to d45de7c Compare April 30, 2024 21:26
@SilasPeters
Copy link
Member Author

I was told that we don't use babel anymore, but lot's of babel dependencies were still found. I don't understand react enough to judge if everything is alright now.

@SilasPeters SilasPeters removed their assignment May 4, 2024
@SilasPeters SilasPeters removed the request for review from Riscky May 4, 2024 11:25
@SilasPeters SilasPeters marked this pull request as ready for review May 6, 2024 18:56
@SilasPeters SilasPeters force-pushed the chore/cleanup-config-files branch 3 times, most recently from db1fd8a to d92d6aa Compare May 6, 2024 19:18
We currently use two packagers: npm and yarn. This is bad, and I thus propose to switch to npm.

Furthermore, in the vite PR we removed some .*rc files, which does not seem nessescary as far as I understand,
and it would be better to keep them.
@SilasPeters SilasPeters force-pushed the chore/cleanup-config-files branch from e0b5fd4 to fcbca18 Compare May 9, 2024 12:35
@SilasPeters SilasPeters self-assigned this May 9, 2024
@SilasPeters
Copy link
Member Author

After talking to @josvanos , we solved everything!

The babel linter and the linter jshint linter are no more, eslint now does everything.
Some old or weird linter rules have been removed.

Lastly, we cleaned up lots of things, and added an editorconfig file.

In another PR I will remove all warnings of the linter, and in a follow-up PR I will create a pipeline that tests if the linter passes.

@stickyPiston
Copy link
Member

stickyPiston commented May 9, 2024

[...] and in a follow-up PR I will create a pipeline that tests if the linter passes.

The CI pipeline proposal in #112 already includes a lint check before building: we simply need to merge this PR's branch into it to get that to work.

.eslintrc.json Outdated Show resolved Hide resolved
.eslintrc.json Outdated Show resolved Hide resolved
@josvanos
Copy link
Member

josvanos commented May 9, 2024

As seen in the build.yml pull request, changing the lint command from 'eslint .' to 'eslint src' would be better. You can remove the ignore dist folder line in the eslintrc this way!

@SilasPeters SilasPeters requested a review from stickyPiston May 10, 2024 09:09
Copy link
Member

@stickyPiston stickyPiston left a comment

Choose a reason for hiding this comment

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

頑張った!

@SilasPeters SilasPeters merged commit ee9e24d into master May 10, 2024
@SilasPeters SilasPeters deleted the chore/cleanup-config-files branch May 10, 2024 19:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file maintainability
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants