Conversation
|
Hello @Necmttn! Great stuff 🚀 |
|
Sounds good thank you 🙌 |
yeaight7
left a comment
There was a problem hiding this comment.
@Necmttn Thanks again for the contribution. The asset shape itself looks aligned with this repo, and the repo-side plumbing is already covered by the existing skill/catalog/requirements validation, so I don't think this needs a CLI/source addition on my side.
One concern should block this PR for now: the documented install command is presented as installing the ax CLI, but the current installer does more than place a CLI binary by default. In particular, curl -fsSL ax.necmttn.com/install | sh defaults to running the follow-up axctl install flow where supported, which can set up local services/watcher/setup behavior. That is more side effect than the skill and README disclose, and this repo's policy for tool-dependent skills needs install steps to be explicit, approval-gated, and reviewable.
Could you update the skill and README row to either use a safer CLI-only install command, for example with --no-run-install, or clearly split the binary install from any later setup/ingest/service steps with separate explicit approval? Once that is made clear, I don't see a need for additional repo CLI changes for this contribution.
Apart from this concern, I don't see any other blocking issue. Just raising this concern to see what's your take on it
Summary
Checks
Generated with ax.