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

Add curated extensions list as source of truth #155

Merged
merged 7 commits into from
Nov 22, 2024

Conversation

damianmarti
Copy link
Member

The extensions.json file will be used from the scaffoldeth.io website to get the curated extensions.

src/extensions.json Outdated Show resolved Hide resolved
src/types.ts Outdated Show resolved Hide resolved
Copy link
Collaborator

@technophile-04 technophile-04 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also for some reason the linting is not working properly.

When I do yarn lint I don't get any error but when if look at src/utils/parse-arguments-into-options.ts file my editor is showing this:

image

@technophile-04
Copy link
Collaborator

Just created #156 for #155 (review)

@damianmarti
Copy link
Member Author

Thanks all for the review!! I removed builder, coBuilders and installCommand. You can add an installCommand if it's different from the default one.

Copy link
Collaborator

@technophile-04 technophile-04 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Summary

Due to the difference in types between create-eth and scaffoldeth.io, we’re already doing some different things in the scaffoldeth.io UI based on the isCurated flag. So, maybe we should keep only these fields in the .json file for create-eth so it's easy for us when we add new extensions to curated list.

// json file type
type Extension = {
  branch: string;
  repository: string;
  // use full for scaffoldeth.io
  description: string;
  version?: string; // (optional) if not present we default latest
  name?: string; // (optional) if not present we use the branch name as default (name might be useful to nicely display challenges names)
  // github: can be inferred by joining repositry + branch (scaffoldeth.io is requiring this for displaying link)
  // installCommand can inferred using repository + branch + version
}[];

Reasoning for above thing:

For cli internal working we need this type:

export type ExternalExtension = {
  repository: string;
  branch?: string | null;
};

export type CLI_TO_WORK = Record<string, ExternalExtension>; // where string is extension name

And scaffoldeth.io is expecting:

type Extension = {
  // Displayed as card title
  name: string;
  // Shown in card body
  description: string;
  // Used for GitHub icon link  (this is ambiguous to `repository` in create-eth but not the same. Here github link contains branch as well)
  github: string;
  // Command shown in copy-to-clipboard button at bottom of card
  installCommand: string;
  builder: string;
  coBuilders: string[];
  youtube?: string;
};

The type which we are maintaining in json for create-eth + scaffoldeth.io:

type Extension = {
  name: string;
  description: string;
  repository: string;
  branch: string;
}[];

@damianmarti
Copy link
Member Author

Summary

Due to the difference in types between create-eth and scaffoldeth.io, we’re already doing some different things in the scaffoldeth.io UI based on the isCurated flag. So, maybe we should keep only these fields in the .json file for create-eth so it's easy for us when we add new extensions to curated list.

// json file type
type Extension = {
  branch: string;
  repository: string;
  // use full for scaffoldeth.io
  description: string;
  version?: string; // (optional) if not present we default latest
  name?: string; // (optional) if not present we use the branch name as default (name might be useful to nicely display challenges names)
  // github: can be inferred by joining repositry + branch (scaffoldeth.io is requiring this for displaying link)
  // installCommand can inferred using repository + branch + version
}[];

Reasoning for above thing:

For cli internal working we need this type:

export type ExternalExtension = {
  repository: string;
  branch?: string | null;
};

export type CLI_TO_WORK = Record<string, ExternalExtension>; // where string is extension name

And scaffoldeth.io is expecting:

type Extension = {
  // Displayed as card title
  name: string;
  // Shown in card body
  description: string;
  // Used for GitHub icon link  (this is ambiguous to `repository` in create-eth but not the same. Here github link contains branch as well)
  github: string;
  // Command shown in copy-to-clipboard button at bottom of card
  installCommand: string;
  builder: string;
  coBuilders: string[];
  youtube?: string;
};

The type which we are maintaining in json for create-eth + scaffoldeth.io:

type Extension = {
  name: string;
  description: string;
  repository: string;
  branch: string;
}[];

Thanks @technophile-04

I removed the name from the JSON file and I'm using the branch as the name if no name is added.

Do we need to add version? Do we need to use a different create-eth version for any curated extension?

@technophile-04 technophile-04 force-pushed the curated-extensions-source-of-truth branch from cad4822 to fe162b3 Compare November 22, 2024 15:49
Copy link
Collaborator

@technophile-04 technophile-04 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to add version? Do we need to use a different create-eth version for any curated extension?

Ohh yeah for speedrunethereum challenges we want the version to locked npx [email protected] -e challenge-0-simple-nft this makes sure challenges always nicely works and then after sometimes we again test new version of npx create-eth with challenges and update the README of challenges.


Thanks @damianmarti!! Just pushed a tweak at fe162b3 just making sure we only use branch as key for internal working of CLI instead of name. The name field is only helpful for scaffoldeth.io to show it as card title so it could be something like name: "Challenge 0: Simple NFT" and we don't want people to pass it as -e flag instead we want them to pass it a branch which is more parsable by cli.

@technophile-04 technophile-04 merged commit d5d30e8 into main Nov 22, 2024
1 check passed
@technophile-04 technophile-04 deleted the curated-extensions-source-of-truth branch November 22, 2024 16:07
@damianmarti
Copy link
Member Author

Do we need to add version? Do we need to use a different create-eth version for any curated extension?

Ohh yeah for speedrunethereum challenges we want the version to locked npx [email protected] -e challenge-0-simple-nft this makes sure challenges always nicely works and then after sometimes we again test new version of npx create-eth with challenges and update the README of challenges.

Thanks @damianmarti!! Just pushed a tweak at fe162b3 just making sure we only use branch as key for internal working of CLI instead of name. The name field is only helpful for scaffoldeth.io to show it as card title so it could be something like name: "Challenge 0: Simple NFT" and we don't want people to pass it as -e flag instead we want them to pass it a branch which is more parsable by cli.

But the branch is set as optional https://github.com/scaffold-eth/create-eth/blob/main/src/types.ts#L7

I thought because the extension could be on another repository using the main branch...

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.

4 participants