-
Notifications
You must be signed in to change notification settings - Fork 860
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 guide documentation #670
Comments
Yep, I like the Wiki idea... where we can write some (meta) stuff about SE-2 dev. But open to other solutions! |
Just tried writing a very basic POC of wiki covering most of the discussion : Demo :Code style guideWelcome to Scaffold-ETH 2 wiki! This wiki holds a summary code style guides that are used in the Scaffold-ETH 2 codebase. The code style is inspired by Google style guide but deviates in some aspects. IdentifiersIdentifiers follows the casing convention in below table:
Import PathsScaffold-ETH 2, nextjs package has path alias While importing use the path alias whenever possible. Example: import { Address } from "~~/components/scaffold-eth";
import { useScaffoldWriteContract } from "~~/hooks/scaffold-eth"; Creating PageDefine the page component and then export it as default. import type { NextPage } from "next";
const Home: NextPage = () => {
return <div>Home</div>;
};
export default Home; CommentsMake comments that actually add information. Example: // BAD:
/*
* Checks if an address is zero address
* @ returns boolean
* */
export const isZeroAddress = (address: string) => address === ZERO_ADDRESS;
// GOOD:
// Checks if an address is zero address
export const isZeroAddress = (address: string) => address === ZERO_ADDRESS; TypescriptTypes vs InterfacesScaffold-ETH 2 uses types over interfaces for defining custom types whenever possible. Types namingTypes should follow the UpperCamelCase convention. custom type should not use the Example: type TAddress = string; // Bad: T prefix is used for generic types
type Address = string; // Good Type InferenceTry to avoid explicitly typing variables unless it's necessary. Example: const x: boolean = true; // Bad: Unnecessary type annotation, TypeScript can infer the type
cc @carletex, @rin-st , @damianmarti, @Pabl0cks |
Great start Shiv, thanks! Had the same todo in my notes, glad I don't need to write it since 100% you did it better 😄
I think comment doesn't add information here since it copies text from variable name. Probably it's better to use for example (similar, but imho not so obvious)
It's just won't work if it's not default, so looks like this point shouldn't be in style guide
I think no 🤷♂️
Probably next/foundry parts when we add foundry and it will be some rules we need to mention. I think we don't need to divide for now
Afaik, we don't have it (except prettier rules)
Link to prettier/eslint configs? Ask to configure IDE so formatters should work? |
We have too little solidity code or hardhat code (and we are planning to change it to Foundry) in SE-2, so I think it is unnecessary now.
thanks @technophile-04 !!! I think this is a really great starting point and we should keep in mind to try to keep it updated to date when we define some new code style or find one missing out here |
found interesting point in Google style guide https://google.github.io/styleguide/tsguide.html#function-declarations. I like it but as I understand most people don't. If you're prefer arrow functions I think we need to add it to our styleguide too |
Description :
We recently did a bunch of clean-up / refactoring code styles PRs to conquer #617 and #232.
Let's document / summarise all the styles that we agreed upon, maybe we can create a Github Wiki as suggested by Carlos here but please free to suggest any other places which feel appropriate 🙌
Some summarised comments :
The text was updated successfully, but these errors were encountered: