Skip to content

Fix publishing workflow and update scripts for web packages#768

Open
jacobsimionato wants to merge 4 commits intogoogle:mainfrom
jacobsimionato:publish
Open

Fix publishing workflow and update scripts for web packages#768
jacobsimionato wants to merge 4 commits intogoogle:mainfrom
jacobsimionato:publish

Conversation

@jacobsimionato
Copy link
Collaborator

These are to use temporarily while set up the automated pipeline properly.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces manual publishing instructions for the web_core package, including necessary configuration files and updates to package.json. The documentation is a good addition for the temporary manual process. However, I've noticed that the new PUBLISHING.md file contains hardcoded version numbers, which can become outdated and lead to errors. I've added a couple of suggestions to replace these with more generic placeholders to improve the long-term maintainability of the document.

Note: Security Review has been skipped due to the limited scope of the PR.


Ensure your local repository is clean and on the correct branch (e.g., `main`).

1. **Check Version:** Ensure the `version` in `package.json` is set correctly. For the current release, it should be `"0.8.0"`.
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

Hardcoding the version number "0.8.0" in the documentation can lead to errors in future releases if a developer forgets to update this line. It's better to provide a general instruction without a specific, hardcoded version.

Suggested change
1. **Check Version:** Ensure the `version` in `package.json` is set correctly. For the current release, it should be `"0.8.0"`.
1. **Check Version:** Ensure the `version` in `package.json` is set to the correct version for the release.

## 6. Post-publish (Optional but recommended)

After a successful publish:
1. Create a git tag for the release (e.g., `git tag v0.8.0`).
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

Similar to the version check, this hardcoded version in the git tag example will become outdated. Using a generic placeholder or a non-specific example is more maintainable and less prone to causing confusion for future releases.

Suggested change
1. Create a git tag for the release (e.g., `git tag v0.8.0`).
1. Create a git tag for the release (e.g., `git tag v1.2.3`).

Copy link
Collaborator

Choose a reason for hiding this comment

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

Can't wait to deprecate this flow :P

@jacobsimionato jacobsimionato changed the title Update manual publishing instructions for web_core Fix publishing workflow and update scripts for web packages Mar 5, 2026
Copy link
Collaborator

@gspencergoog gspencergoog left a comment

Choose a reason for hiding this comment

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

Thanks for writing this up!

@@ -0,0 +1,85 @@
import { readFileSync, writeFileSync, copyFileSync, mkdirSync, existsSync } from 'fs';
Copy link
Collaborator

Choose a reason for hiding this comment

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

Copyright?

Copy link
Collaborator

@ditman ditman left a comment

Choose a reason for hiding this comment

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

Thanks for getting the markdown-it to a publishable state, this is fantastic!

Small comment about the dependencies for web_core. The whole build post-processing happened because web_core got pushed to the repo without being published to npm. Now that the package is published, we don't need any of that. Local development should happen with npm link or workspaces (I don't know yet :P)

@@ -38,6 +38,12 @@ if (!packageJson.dependencies['@a2ui/web_core']) {
}

packageJson.dependencies['@a2ui/web_core'] = '^' + coreVersion;
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we should stop post-processing the web_core dependency, and instead, take a dependency on the version of the package that's published on npm. That way, these build script could be simplified quite a lot! WDYT?

Then, we'd need some instructions on how to develop locally to be able to modify multiple packages at once, with workspaces, or npm link (?)

Copy link
Collaborator

Choose a reason for hiding this comment

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

my 2¢ is a npm link workflow is probably the best way to approach this, whenever we document that.

// 3. Adjusting paths in package.json (main, types, exports) to be relative to dist/

const dirname = import.meta.dirname;
const corePkgPath = join(dirname, '../../web_core/package.json');
Copy link
Collaborator

Choose a reason for hiding this comment

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

This package is going to be fairly stable, and it most likely won't need a lot of changes to web_core. Can you please update the package.json to take a dependency on the published version of web_core? Maybe we can remove most of this file?

{
"name": "@a2ui/web_core",
"version": "0.8.2",
"version": "0.8.0",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Would it make sense to call this fix 0.8.1 and publish the package, so the package.json version matches what's published (not retroactively? :P)

@yourkarma6788

This comment was marked as spam.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Todo

Development

Successfully merging this pull request may close these issues.

5 participants