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

Code style preferences and guidelines #82

Open
puffo opened this issue Sep 6, 2022 · 5 comments
Open

Code style preferences and guidelines #82

puffo opened this issue Sep 6, 2022 · 5 comments

Comments

@puffo
Copy link
Contributor

puffo commented Sep 6, 2022

While we extend and cleanup some areas of the codebase, we should start to discuss some of our coding style preferences to prepare for some kind of contributions.md file in future.

This issue can be a place for us to discuss our preferences.

Ongoing List of Standards:

Function signatures

  1. When creating a function that adheres to a TS type, avoid restating the already defined parameter types in the new function.
  2. When creating a function where the context for input parameters is clear, use shorts name for those parameters.

General

  1. Avoid leaving TODOs in the codebase. Rather generate a Github issue with a decent description.
@puffo
Copy link
Contributor Author

puffo commented Sep 6, 2022

We define MapTrack in process.ts:

export type MapTrack = (nft: NFT, apiTrack: any, nftFactory?: NftFactory | undefined, trackId?: string) => ProcessedTrack;

But then we also re-specify the types when implementing it. e.g. nina.ts

const mapTrack: MapTrack = (
  nft: NFT,
  apiTrack: any,
  contract?: NftFactory,
  trackId?: string,
): ProcessedTrack => { // ...

What do we think about cleaning things up a bit and leaning on the TS types instead?

const mapTrack: MapTrack = (
  nft,
  apiTrack,
  contract?,
  trackId?,
) => { // ...

An example of what this cleanup looks is here: e0f4be1

@musnit
Copy link
Contributor

musnit commented Sep 6, 2022

ye sounds great

@musnit
Copy link
Contributor

musnit commented Sep 6, 2022

the linked changes look good too

@puffo
Copy link
Contributor Author

puffo commented Sep 7, 2022

When creating a function where the context for its parameters is self evident, prefer short names instead of being overly specific.

Style recommendation based on discussion in https://github.com/spinamp/spindexer/pull/76/files#r965095193

@puffo
Copy link
Contributor Author

puffo commented Sep 7, 2022

We don't make use of TODOs in the codebase, prefer github issues or just tackle the problem if it isn't a major change!

Source:
https://github.com/spinamp/spindexer/pull/76/files#r965104227

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

No branches or pull requests

2 participants