Skip to content
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

add script to fix package json from build step #417

Merged
merged 2 commits into from
Apr 28, 2023

Conversation

oscard0m
Copy link
Member

@oscard0m oscard0m commented Apr 28, 2023

Relates to octokit/plugin-retry.js#410 (comment)


  • fix(ci): add hotfix in built package.json to use proper file patterns in "files"
  • ci(release): run 'scripts/fix-package-json.js' before release

Behavior

Before the change?

The released npm package is missing most of the files generated by the build step. dist-node, dist-types, dist-web... even though they are generated correctly.

You can read more about my explanation in the linked issue.

After the change?

I expected npm to read the file patterns correctly so we publish all the necessary files again.

Other information

This is a mix of an issue with npm@v9 (npm/cli#6164) and the fact we rely on pika for the build step. Pika has been archived since April 2022 so there is nothing we can do with Pika.

I'm opening a discussion to discuss what we should do: octokit/octokit.js#2403

Open questions

If we agree on this solution, we need to plan:

  • How to merge and release this?
  • What do we do with the rest of the repositories?
  • What do we do with the already published versions with missing files?

Additional info

Pull request checklist

Because this is kind of a temporary hack, do you think I should add tests + documentation for this?

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been reviewed and added / updated if needed (for bug fixes / features)
  • Added the appropriate label for the given change

Does this introduce a breaking change?

No

Pull request type

Because of the problems is giving to users, I'm treating it as a bug: Type: Bug. In terms of semantic commit, let me know if I need to change ci() to fix()


@oscard0m oscard0m added Priority: High Type: Bug Something isn't working as documented labels Apr 28, 2023
@oscard0m oscard0m requested review from wolfy1339, a team, nickfloyd and kfcampbell and removed request for a team April 28, 2023 13:12
@wolfy1339 wolfy1339 merged commit 1a06c5f into main Apr 28, 2023
@wolfy1339 wolfy1339 deleted the add-script-to-fix-package-json-from-build-step branch April 28, 2023 14:08
@oscard0m
Copy link
Member Author

@wolfy1339 the PR title was not semantic-release compliant. I was going to fix that in the commit message:

https://github.com/octokit/app.js/actions/runs/4831503369/jobs/8609041736

What's the cleanest way to fix this?

@wolfy1339
Copy link
Member

There wasn't a release on the main branch, only beta, so it's not critical.

Another PR with an empty commit to trigger a release, and point back to this one

@github-actions
Copy link

🎉 This PR is included in version 13.1.3 🎉

The release is available on:

Your semantic-release bot 📦🚀

wolfy1339 pushed a commit that referenced this pull request May 21, 2023
* fix(ci): add hotfix in built package.json to use proper file patterns in "files"

* ci(release): run 'scripts/fix-package-json.js' before release
@github-actions
Copy link

🎉 This PR is included in version 14.0.0-beta.4 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
released on @beta released Type: Bug Something isn't working as documented
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants