-
Notifications
You must be signed in to change notification settings - Fork 0
Introducing publish example #4
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Comment @cursor review or bugbot run to trigger another review on this PR
ec84e7d to
271707c
Compare
examples/publish/index.ts
Outdated
| @@ -0,0 +1,87 @@ | |||
| import assert from "node:assert"; | |||
| import { confirm, input } from "@inquirer/prompts"; | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
though: I think, for simplicity, I would drop the CLI, Inquirer, and Interval parts and leave it as a pure framer-api code example for change detection and publishing, and optionally include a cron job config for a GitHub Actions workflow that executes this script every 10 minutes or so.
Can we drop the confirmation step and replace it with a --publish flag?
My concern is that it requires extra effort to understand what is happening here, and having all the items mentioned above also makes it hard to understand the framer-api use case we are trying to demonstrate. So I think it is better to have a simple flat code as possible (no branching with ifs and process.exits)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, I'll strip it all to the core functionality and me have the interval and automation part covered in the readme (cron, systemd, GH Actions).
examples/publish/index.ts
Outdated
|
|
||
| const shouldStart = | ||
| intervalFromEnv || | ||
| (await confirm({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
issue: What is the point of running this script and then asking for approval right away without even checking for changes? Before reading the logic of this script I thought that it would be asking for confirmation before actually publishing which would make more sense to me 🤓
I would prefer to not have the confirmation step in any form in the example that shows this use case
IF we have changes THEN publish+deploy ELSE exit
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did that to be sure the users of it are clear this will publish it in an interval. So that those who just run it, don't accidentally publish it in an interval. But: with the stripped variant, that will no longer be needed anyway.
examples/publish/README.md
Outdated
| @@ -0,0 +1,38 @@ | |||
| # Publish CLI | |||
|
|
|||
| A CLI tool that continuously monitors a Framer project for changes and automatically publishes and deploys them at a set interval. | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
question: Is this something we're gonna use? I think that's a cool idea to have some CLI example. However, I don't think it is a great use for publishing with the interval 😆 and also makes it hard to see the main methods in action, because they are hidden behind CLI code setup
271707c to
ff508fd
Compare
…ample package.json files
ff508fd to
9dc79c9
Compare
Introducing a script that runs at a set interval to log and publish the changes of a project.