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

chore(plugins): unify package json fields #3714

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

outSH
Copy link
Contributor

@outSH outSH commented Jan 3, 2025

  • Unify common fields among all cacti plugin package.json files.
  • Use Cacti instead of Cactus whenever possible.
  • Use current github URL and mailing list addresses.
  • Add publishConfig access public to all packages.

Prerequisite for #3670 where I plan adding a CI check that will assert that all package.json files are correct.

I'm using the following Joi schema as our standard:

const schema = Joi.object({
  name: Joi.string()
    .pattern(new RegExp("^@hyperledger(-cacti)?/.*"))
    .required(),
  version: Joi.string().valid("2.1.0").required(),
  private: Joi.bool().valid(false),
  description: Joi.string().min(10).required(),
  keywords: Joi.array()
    .items(Joi.string())
    .has(Joi.valid("Hyperledger"))
    .has(Joi.valid("Cacti"))
    .required(),
  homepage: Joi.string()
    .valid("https://github.com/hyperledger-cacti/cacti#readme")
    .required(),
  bugs: Joi.object()
    .valid({
      url: "https://github.com/hyperledger-cacti/cacti/issues",
    })
    .required(),
  repository: Joi.object()
    .valid({
      type: "git",
      url: "git+https://github.com/hyperledger-cacti/cacti.git",
    })
    .required(),
  license: Joi.string().valid("Apache-2.0").required(),
  author: Joi.object()
    .valid({
      name: "Hyperledger Cacti Contributors",
      email: "[email protected]",
      url: "https://www.lfdecentralizedtrust.org/projects/cacti",
    })
    .required(),
  files: Joi.array().items(Joi.string()), // required? dist?
  engines: Joi.object()
    .valid({
      node: ">=18",
      npm: ">=8",
    })
    .required(),
  publishConfig: Joi.object({ access: Joi.string().valid("public") })
    .unknown()
    .required(),
}).unknown();

Pull Request Requirements

  • Rebased onto upstream/main branch and squashed into single commit to help maintainers review it more efficient and to avoid spaghetti git commit graphs that obfuscate which commit did exactly what change, when and, why.
  • Have git sign off at the end of commit message to avoid being marked red. You can add -s flag when using git commit command. You may refer to this link for more information.
  • Follow the Commit Linting specification. You may refer to this link for more information.

Character Limit

  • Pull Request Title and Commit Subject must not exceed 72 characters (including spaces and special characters).
  • Commit Message per line must not exceed 80 characters (including spaces and special characters).

A Must Read for Beginners
For rebasing and squashing, here's a must read guide for beginners.

- Unify common fields among all cacti plugin package.json files.
- Use Cacti instead of Cactus whenever possible.
- Use current github URL and mailing list addresses.
- Add publishConfig access public to all packages.

Prerequisite for hyperledger-cacti#3670 where
I plan adding a CI check that will assert that all package.json files are correct.

Signed-off-by: Michal Bajer <[email protected]>
@outSH
Copy link
Contributor Author

outSH commented Jan 3, 2025

@petermetz As mentioned in the commit messasge, this is part of #3670 where I plan to add common package json fields check. I've followed your recommendation to use Joi, please have a look at proposed schema and let me know if you'd like to change / add anything to it. I also have the following questions:

  • I use version check that is hardcoded in a test file (will move it a a const to the top of the file to make it easier to change). Would you like another solution that will cause less troubles during release? Environment variable, maybe add version field to the top package.json file that will be used as a reference when verifying the child packages? I don't want this to be yet another burden for you during release :)
  • I wanted to require that all packages (at least in packages/) are set to public (i.e. not private: true) but I found there is package that break it (cactus-test-plugin-keychain-memory). Shall we change it to public, move it to another directory, or leave as it is and don't check for private in the CI check? I assume it's OK to have private packages in examples/

@VattiPraveen @sandeepnRES I've added...

  "publishConfig": {
    "access": "public"
  },

... to your package.json files in weaver, is that OK? I think that was required by @petermetz at least for cacti-native packages at some point (if I recall correctly), but I'm not sure if it doesn't affect weaver release process in any substantial way. I don't think it should, but please confirm it before the merge so I don't mess up your build by accident.

Copy link
Contributor

@jagpreetsinghsasan jagpreetsinghsasan left a comment

Choose a reason for hiding this comment

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

LGTM
@petermetz when are we planning to change the package names themselves? (from cactus-cmd-api-server to cacti-cmd-api-server...)

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.

2 participants