Skip to content

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 a team, kfcampbell, nickfloyd and wolfy1339 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.

3 participants