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

Add CI for 2.x #405

Merged
merged 5 commits into from
Nov 12, 2023
Merged

Add CI for 2.x #405

merged 5 commits into from
Nov 12, 2023

Conversation

lieser
Copy link
Owner

@lieser lieser commented Nov 7, 2023

This adds a ESLint and Type Check for the 2.x branch.

Enabling and fixing strict type checking is left for a follow up.

@lieser lieser added this to the 2.3.0 milestone Nov 7, 2023
@lieser lieser self-assigned this Nov 7, 2023
@lieser
Copy link
Owner Author

lieser commented Nov 7, 2023

@dodmi This is my proposal for a first step to improve the development experience for the 2.x branch.

I personally think this checks are useful. But it does mean sometimes one has to put a little more work into it. To have code that does not just functions, but there the checkers are also happy.

Please let me know what you thing about this.
Would definitely need some testing by you before I would like to merge it.

Also what IDE are you using? This all works nicely together with VS Code and a local Node.JS installation. Getting type hints (and errors) directly in the IDE.
Example for some code you added a while ago:
grafik
But should also be usable just from the command line.

@dodmi
Copy link
Contributor

dodmi commented Nov 9, 2023

As always, thank you :)
The problem is, I have no experience with Type Script and ESlint, I'm not a developer.
For useful testing, you'll need useful tests and I'm far away from writing such tests. I think, I'll probably need a lot of time to get into it.

@lieser
Copy link
Owner Author

lieser commented Nov 11, 2023

The problem is, I have no experience with Type Script and ESlint, I'm not a developer.

That is not a problem as long as you are willing to learn.

With the help of JSDoc comments and the Type Script compiler you can make Java Script feel more like a statically typed language.

The advantages are:

  • Easier to find errors there variables have a different type than you may have expected.
  • Better development experience because the IDE can show you what type a variable has or that type a function expects as a parameter. See e.g. my screenshot above.

See also https://www.typescriptlang.org/docs/handbook/intro-to-js-ts.html and https://code.visualstudio.com/Docs/languages/javascript#_type-checking.

ESLint (https://eslint.org/) statically analyzes the code to:

  • Find potential problems
  • Enforce some best practices for writing code

As you don't seem to have any objections I will merge this now.
My recommendation would be for you to start using VS Code if you are not doing that already. And just try out the ESlint and type checking integration, and hopefully see how it improves your development experience.

  1. Install VS Code https://code.visualstudio.com/
  2. Install Node.js https://nodejs.org
  3. Run npm ci in the root of the repository to install Node.js dependencies
  4. Open the folder in VS Code
  5. Install the recommended extensions (at least https://marketplace.visualstudio.com/items?itemName=dbaeumer.vscode-eslint)
  6. Start developing

@dodmi
Copy link
Contributor

dodmi commented Nov 12, 2023

Well, I'm really interested and would like to learn. So, I'll give it a try. I'm not using "VS Code". As I'm no developer, I have no full blown IDE, but only Notepad++, which provides syntax highlighting and a function list. So, I'll need some time to get into it (thank you for the links and quick steps to get it working)

@lieser
Copy link
Owner Author

lieser commented Nov 12, 2023

Yesterday I found some errors then I wanted to merge it, fixed it now (and force pushed this branch).

@lieser lieser merged commit c736fe4 into 2.x Nov 12, 2023
4 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