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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move @actions/http-client into the toolkit #1062

Merged
merged 4 commits into from May 3, 2022
Merged

Conversation

brcrista
Copy link
Contributor

@brcrista brcrista commented Apr 25, 2022

馃挕 See #1064 for a better diff!

https://github.com/actions/toolkit contains a variety of packages used for building actions. https://github.com/actions/http-client is one such package, but lives outside of the toolkit. Moving it inside of the toolkit will improve discoverability and reduce the number of repos we have to keep track of for maintenance tasks (such as github/c2c-actions-service#2937).

I checked with @bryanmacfarlane on the historical decision here. Apparently it was just inertia from before we released the toolkit as multiple packages.

The benefits here are:

  • Have one fewer repo to keep track of
  • Signal that this is an HTTP client meant for building actions, not for general use.

Notes

  • @actions/http-client will continue to be released as its own package.
  • Bumping the package version to 2.0.0. Since we're compiling in strict mode now, there are some breaking changes to the exported types. This is an improvement because the null-unsafe version ofhttp-client is currently breaking the safety of null-safe consumers.
  • I'm not updating the other packages to use the new version in this PR. I plan to do that in a follow-up. We'll hold off on publishing http-client v2 to NPM until that's done just in case other changes shake out of it.

fix failing http-client tests

npm run lint-fix

Simple manual fixes for lint rules

Silence some linter errors

Silence some linter warnings, but not "explicit any"

Silence @typescript-eslint/no-explicit-any

fix @typescript-eslint/no-non-null-assertion

fix npm audit for http-client

don't use I- prefix on interface names

Get rid of `any` type in `Headers`

Add http-client to the main README

Update release notes
@@ -9,5 +9,5 @@ if [[ -z "$name" ]]; then
exit 1
fi

lerna create @actions/$name
cp packages/toolkit/tsconfig.json packages/$name/tsconfig.json
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This file doesn't exist

Copy link
Collaborator

@thboop thboop left a comment

Choose a reason for hiding this comment

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

lgtm

@brcrista brcrista merged commit 91b7bf9 into main May 3, 2022
brcrista added a commit that referenced this pull request May 11, 2022
We moved `@actions/http-client` to be part of the toolkit in #1062.  We also made some breaking changes to exported types and released v2.

The biggest change in terms of lines of code affected was to get rid of the `I-` prefix for interfaces since TypeScript doesn't follow this convention.

I bumped the patch version of all packages except for `tool-cache`, where I bumped the major version.  The rationale is explained in the release notes for that package.
@karlhorky
Copy link

karlhorky commented May 13, 2022

@thboop @brcrista seems like moving tunnel to devDependencies caused errors like Error: Cannot find module 'tunnel' (eg. when upgrading to @actions/core@1.8.1):

#1083

Apparently this should be fixed in #1084

@brcrista brcrista deleted the brcrista/http-client branch May 16, 2022 16:56
@brcrista
Copy link
Contributor Author

Thanks @karlhorky, should be fixed now #1085

worldlight425 added a commit to worldlight425/toolkit that referenced this pull request Feb 22, 2023
We moved `@actions/http-client` to be part of the toolkit in actions/toolkit#1062.  We also made some breaking changes to exported types and released v2.

The biggest change in terms of lines of code affected was to get rid of the `I-` prefix for interfaces since TypeScript doesn't follow this convention.

I bumped the patch version of all packages except for `tool-cache`, where I bumped the major version.  The rationale is explained in the release notes for that package.
at-wat pushed a commit to at-wat/actions-toolkit that referenced this pull request Aug 31, 2023
We moved `@actions/http-client` to be part of the toolkit in actions#1062.  We also made some breaking changes to exported types and released v2.

The biggest change in terms of lines of code affected was to get rid of the `I-` prefix for interfaces since TypeScript doesn't follow this convention.

I bumped the patch version of all packages except for `tool-cache`, where I bumped the major version.  The rationale is explained in the release notes for that package.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants