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

Improve parser #5

Merged
merged 10 commits into from
Nov 29, 2022
Merged

Improve parser #5

merged 10 commits into from
Nov 29, 2022

Conversation

scripthunter7
Copy link
Member

@scripthunter7 scripthunter7 commented Nov 27, 2022

Minor modifications based on previous reviews.

  • CamelCase
  • Reorganization of constants

TODO:
@maximtop How do I handle the examples below? Should they follow the "Ublock" pattern too? I think uBO is named inconsistently in a few more places. This should be unified in this PR.

@maximtop
Copy link
Contributor

I suggest to write like this

Ublock
Adguard
Adblock

ublock
adguard
adblock

or short versions

Ubo
Adg
Abp

ubo
adg
abp

export interface IuBlockModifierList {

It is recommended not to use prefix "I" in the interface name
https://stackoverflow.com/questions/31876947/confused-about-the-interface-and-class-coding-guidelines-for-typescript/41967120#41967120

@scripthunter7
Copy link
Member Author

Ok, thanks. I will make these changes.

  • I will probably use short names to keep the method names more compact.
  • I take out the "I" prefixes
  • I also include these two rule configurations in ESLint:
    "jsdoc/require-param-type": 0,
    "jsdoc/require-returns-type": 0
    
    In this way, there is no need to write out the types repeatedly in the JSDoc. You mentioned this in a previous review, that it is unnecessary, since the types are revealed from the function's signature anyway.

@scripthunter7
Copy link
Member Author

scripthunter7 commented Nov 28, 2022

@maximtop

This is a relatively large commit, sorry.

The linter shows no problems and the tests run.

@scripthunter7
Copy link
Member Author

scripthunter7 commented Nov 28, 2022

Build also improved: a75aec9 #6

rollup.config.ts Outdated Show resolved Hide resolved
@scripthunter7 scripthunter7 merged commit 19653d9 into master Nov 29, 2022
@scripthunter7 scripthunter7 deleted the improve-parser-1 branch February 20, 2023 21:27
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