-
Notifications
You must be signed in to change notification settings - Fork 30
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
split up into two packages #193
Conversation
I've done a test release as
|
inputs: | ||
version: | ||
description: "The version to publish" | ||
required: true |
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.
This workflow now needs to be manually triggered from the Github Actions UI, as we need to be very specific about the version that will be applied to all packages (and parsing some release title for a version number might turn out flimsy).
This also allows us to publish under a certain tag.
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.
Would it be worth spending some effort getting changesets integrated? It handles workspaces with dependent packages pretty well from what I've seen (check our brand repo for an example, specifically the tailwind package that depends on the base brand
package)
@@ -1,5 +1,6 @@ | |||
name: Snapshot Release | |||
on: | |||
workflow_dispatch: |
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.
This workflow can now also be triggered from the GitHub UI, which has the benefit that it will actually run the workflow version of that branch and not the workflow from main
- meaning, we can not test it before merging into main
.
@@ -1,5 +1,5 @@ | |||
"use client"; | |||
import { useContext, useEffect, useState } from "rehackt"; | |||
import { useContext, useEffect, useState } from "react"; |
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.
With all the exports conditions we have in play for these packages now, we don't need rehackt
anymore.
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.
💯 🔥 💯 Nice work!
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.
[nit] Should the workspace name match the package name? In other words, would it be alright to drop the -support
suffix from this folder name?
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.
Can do :)
617624c
to
4c5565e
Compare
4c5565e
to
fb2bc7e
Compare
No description provided.