feat: add status and statusMessage to InstructionNode

[tanmay4l]

#263 cc @lorisleiva

[changeset-bot]

🦋 Changeset detected

Latest commit: de9dff3

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 12 packages

Name	Type
@codama/visitors-core	Minor
@codama/node-types	Minor
@codama/nodes	Minor
@codama/cli	Patch
@codama/dynamic-codecs	Patch
@codama/dynamic-parsers	Patch
@codama/renderers-core	Patch
@codama/validators	Minor
@codama/visitors	Minor
@codama/errors	Minor
codama	Minor
@codama/nodes-from-anchor	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[lorisleiva]

Hey, thanks for tackling this! How would you feel about creating a new InstructionStatusNode instead that attaches to the InstructionNode in an optional status attribute?

Then we can have a helper in @codama/nodes like status: InstructionStatusNode("deprecated", "Some deprecated message").

We'll also need to update core visitors so that the new node is registered properly and release a matching PR in codama-rs repo so that the standard is kept in-sync in both languages.

Finally, I think we can drop the "unaudited" status for now. This was a passing thought I had and thinking about it more I think the audit status should probably be more at the program level than the instruction level (something we can work on some other time).

[tanmay4l]

Hey, thanks for tackling this! How would you feel about creating a new InstructionStatusNode instead that attaches to the InstructionNode in an optional status attribute?

Then we can have a helper in @codama/nodes like status: InstructionStatusNode("deprecated", "Some deprecated message").

We'll also need to update core visitors so that the new node is registered properly and release a matching PR in codama-rs repo so that the standard is kept in-sync in both languages.

Finally, I think we can drop the "unaudited" status for now. This was a passing thought I had and thinking about it more I think the audit status should probably be more at the program level than the instruction level (something we can work on some other time).

yep sounds good to me so status can be transformed/validated separately

[tanmay4l]

hey @lorisleiva created InstructionStatusNode as a separate node attached to InstructionNode as optional status added the helper in @codama/nodes and updated all core visitors. also got the matching codama-rs pr ready will raised later

one que changeset bot flagged missing changeset do i need to add one or is that handled separately?

[lorisleiva]

Hey! Sorry for the late review, I wanted to take my time to review this thoroughly.

I've made a couple of comments but structurally I think this is good!

Regarding changeset, yes could you please run changeset locally and follow the prompts? It'll ask you which packages are being affected by this PR (should auto-suggests the ones that have changes for you) and it will then ask for a bump level and a message.

I'd stay we go for a minor bump here because it's not a breaking change but it's a new feature.

Let me know when you have the Rust PR ready as well please. 🙏

Thanks again for you help with this!

[tanmay4l]

@lorisleiva, yes, thank you for the review I’ll run the changeset locally and check it out. Btw the Rust PR is ready as well. I haven’t raised a bcs because I want your feedback, but here it is - codama-idl/codama-rs#75

[lorisleiva]

Tysm! Just a couple of small comments and we're there.

EDIT: One of the tests is failing as well I think because of a status residue somewhere.

[lorisleiva]

For the lint issues, you should be able to run pnpm lint:fix on the root directory.

[tanmay4l]

For the lint issues, you should be able to run pnpm lint:fix on the root directory.

Linting should be fixed now :)

[lorisleiva]

Thank you!