ubiquity / ts-template

A template repository for all @ubiquity projects.
2 stars 24 forks source link

Implement Pascal Case for Erc20Permit and Erc721Permit #40

Closed jordan-ae closed 6 months ago

jordan-ae commented 7 months ago

… naming conventions

Resolves #39

github-actions[bot] commented 7 months ago
Lines Statements Branches Functions
Coverage: 80%
80% (4/5) 100% (0/0) 66.66% (2/3)

JUnit

Tests Skipped Failures Errors Time
1 0 :zzz: 0 :x: 0 :fire: 2.74s :stopwatch:
Coverage Report (80%)
File% Stmts% Branch% Funcs% LinesUncovered Line #s
All files8010066.6680 
   main.ts8010066.66809
jordan-ae commented 7 months ago

You also should not change any other files.

@0x4007 The other modifications were made my running the linters. If you check the changes you'll see the linters just added spaces where it seemed necessary and all that. I can revert the changes if they will cause any issues though.

gentlementlegen commented 7 months ago

There actually already is a rule for PascalCase interfaces, no need to create a new one. The thing is that rule enforces Pascal Case but not strictly meaning you can chain uppercase letters.

If we want to prevent this we can use StrictPascalCase. Maybe it would be a good idea to use it everywhere @0x4007 ? See also: https://typescript-eslint.io/rules/naming-convention/#format-options

0x4007 commented 7 months ago

That rule seems to only check for the first character being capitalized according to the docs. I think a regex that looks for three or more consecutive capitalized letters should be flagged as an error

gentlementlegen commented 7 months ago

StrictPascalCase will enforce no more that one capital letter inside the name, regardless of the position. Example:

image

Quoting the docs: "consecutive capitals are not allowed (i.e. myId is valid, but myID is not)."

0x4007 commented 7 months ago

I think let's use strictpascalcase then!

gentlementlegen commented 7 months ago

@0x4007 Shall this be applied to other elements too, like variables etc? There is the same version for camel case called StrictCamelCase.

0x4007 commented 7 months ago

Yes let's apply this across all identifiers.

jordan-ae commented 7 months ago

@0x4007, @gentlementlegen I've added StrictPascalCase to the eslint rules. They'll be a couple of errors in certain files due to the new eslint configuration which I haven't changed because of this comment. I'd love to clean up the files though!

You also should not change any other files.

I also suggest we create a pre-commit hook. By doing so we can ensure no errors go into the repository and enforce code style. The pre-commit hook will run only on staged files to avoid slow and irrelevant processes.

0x4007 commented 7 months ago

We have lint-staged for this purpose but perhaps it's not configured correctly.

jordan-ae commented 7 months ago

We have lint-staged for this purpose but perhaps it's not configured correctly.

Okay I'll take sometime to look into that too

jordan-ae commented 7 months ago

@gentlementlegen please can you review? Thanks

ubiquity-os-deployer[bot] commented 7 months ago
66e9fe8
850886a
bad8429
jordan-ae commented 6 months ago

Hey @0x4007 I've undone all the changes not related to the issue. This should be ready to go now!

jordan-ae commented 6 months ago

I think let's use strictpascalcase then!I

I thought we were going full in on StrictPascalCase 😬