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

Solves #22 and greatly improves the project documentation #23

Merged
merged 5 commits into from
May 31, 2024

Conversation

Sharcoux
Copy link

Add types for use with intellisense, typescript, etc, and improve project documentation

Solve #22

Copy link
Owner

@derhuerst derhuerst left a comment

Choose a reason for hiding this comment

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

Can you keep the formatting (no trailing semicolons, indentation, etc.) as before? The current changeset makes reviewing (and blaming in the future) very hard.

@Sharcoux
Copy link
Author

You should probably define those in a linter config, this way the formatting will always follow the same rules.

@derhuerst
Copy link
Owner

derhuerst commented May 30, 2024

You should probably define those in a linter config, this way the formatting will always follow the same rules.

I see your point! But this creates more maintenance work, and I would like to rather make it less. See also public-transport/hafas-client#305 (comment) .

@Sharcoux
Copy link
Author

This is the opposite. The linter will automatically ensure that the code style is the same throughout the project. I don't like prettier either, but you can perfectly lint code with eslint, which is already installed in the project. If you check my last change, it implements some of your requirements.

index.js Outdated Show resolved Hide resolved
@derhuerst
Copy link
Owner

Adding a linter creates long-term friction for me, this is why I don't want to have such a setup. (I can see that using it is worth it in projects with multiple active maintainers and/or many contributors.)

Thanks for adjusting your PR accordingly.

@Sharcoux
Copy link
Author

Adding a linter creates long-term friction for me, this is why I don't want to have such a setup. (I can see that using it is worth it in projects with multiple active maintainers and/or many contributors.)

Thanks for adjusting your PR accordingly.

You already have a linter. eslint is a linter - there is a hint in the name 😉

@derhuerst derhuerst merged commit 734be40 into derhuerst:master May 31, 2024
3 checks passed
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