An approach to technical debt

[cacieprins]

Hello! I know I just came out of the blue, and don't have many contributions yet. Please don't take this as a personal criticism! All projects accrue technical debt over time, and I actually love fixing it. I've also found that one of the best ways to really get to know a new codebase is to start with small technical debt: this usually ends up touching quite a few pieces of the application, and requires a deep understanding of how things fit together to go smoothly. In respect to that (and in respect to ya'll maintainers, so you can continue adding new features), I'd like to propose this blueprint for my contributions:

Phase One: Non-invasive Eslint Flags

Work through the backlog of disabled eslint flags that can be fixed without major refactoring. This is pretty much all of them except ban-types, explicit-module-boundary-types, and no-explicit-any. The order in which they're done doesn't really matter, but for easy reference:

• ban-types: this is only partial, for components that expect an empty props object. Some other places, like component states, require full API type definitions
• no-empty-function
• no-empty-interface
• no-namespace
• no-this-alias
• triple-slash-reference
• no-empty-pattern
• no-prototype-builtins
• no-self-assign
• no-useless-escape
• prefer-spread

As a part of this, I propose adding an explicit exception for no-misleading-character-class for lib/misc.ts#449 as it seems like that unicode filter is pretty important, and I'm not familiar enough with working w/ unicode text manipulation to fix it yet :) this is a pretty important one to keep around and enable, though, as it can detect certain kinds of obfuscated malicious code.

This is a good first pass to get me familiar with the codebase.

Phase Two: Complete Typescript Migration

This is a multi-step plan to work towards re-enabling the ban-types, no-explicit-any, and explicit-module-boundary-types eslint flags.

1. Complete the API interfaces in src/models. Some of the API schema is missing from apiary, unfortunately. One of the most important field attributes that I couldn't glean from apiary on the endpoints that are documented, is which are nullable. Do you need help fleshing out the api docs?
2. Add type hinting to calls to fetch/request/etc. This sets the stage for working through types that cannot be inferred. For example, get("tournaments/%%/rounds", tournament_id) as Promise<TournamentRounds[]> on L274 of src/views/Tournament/Tournament.tsx would cascade through the rest of the component and resolve quite a few of the eslint errors.
3. Fix any left-over no-explicit-any errors
4. Fix any left-over explicit-module-boundary-types

What do you think? OGS is super cool, and overall this is a really interesting codebase. I'd love to help enable the main contributors to iterate faster on new features by backfilling some of this stuff.

[anoek]

Sure thing, @benjaminpjones and @maurelio1234 have been working on those kinds of things a fair amount, so they might have some comments, but what you describe is the general direction we're all heading, so welcome, and we look forward to some PRs to help us get things in tip top shape!

[maurelio1234]

Hi! This looks like #1629 where I tried to help a bit.
It seems to be the same kind of idea, enable the rule, fix a couple of errors, then create a PR, repeat... until the rule can be permanently enabled.

I'd be happy to help anyway ;)

Another thing that could be tested is something like https://github.com/okonet/lint-staged .
So that the new rules could apply on new code.
But I didn't have time to test it nor I ever used this project.

[benjaminpjones]

Rock on, these all sound like good ideas! I really relate to fixing tech debt to onboard to a new codebase :)

Do you need help fleshing out the api docs?

Yes haha, I think @shinuito might have some comments? I know he is currently working on/has recently migrated rest api docs to swagger.

1. Complete the API interfaces in src/models.

BTW if you have any ideas on how to organize stuff, feel free to change existing definitions. That directory is pretty new and I've just been adding things organically. I think it can probably be structured more intuitively.

[cacieprins]

Oh heck yes, swagger docs would actually help a TON. There are actually a lot of tools out there for generating entire typedefs from your swagger/openapi schema.

[shinuito]

I'm submitting a pull request with what I have so far for the get endpoints. #1768 I might still need to run prettier and I've left some comments there about the post endpoints needing to be tested and documented, and then the schema and examples for the returned data.

But anyway, once something is there to start with and the api docs website is generate from those docs, then it should be easy for everyone to contribute as well. I kind of want to look at one or two more things before I continue with the docs, so maybe made sense to push what I have so far.