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

v4 w/ TypeScript Overhaul #112

Merged
merged 1 commit into from
Aug 7, 2023
Merged

Conversation

JacobMGEvans
Copy link
Contributor

@JacobMGEvans JacobMGEvans commented Jul 6, 2023

  • The Wrangler Action project on GitHub is being modernized due to lack of maintenance.
  • The project will be rewritten in TypeScript, removing dependencies such as Docker.
  • Docker's removal will decrease spin-up time.
  • Community-requested features, including bulk secrets API utilization from Wrangler, will be added.
  • The project's CI/CD will be fixed and testing added.
  • The command implementation will be improved.
  • Documentation will be updated to reflect changes.
  • The project will begin using Node for the Action engine/runner.
  • All changes will be openly discussed with the community.

Additional related Internal tickets:
Major Version Default: https://jira.cfdata.org/browse/DEVX-632
Rewrite Project: DEVX-804,802,800,632

@JacobMGEvans JacobMGEvans requested a review from a team July 14, 2023 20:03
@winstxnhdw
Copy link

winstxnhdw commented Jul 14, 2023

Hey @JacobMGEvans, super cool to see a TypeScript re-write for wrangler-actions! Regarding #105, what do you think of adding a quiet parameter, which when set to true, passes { stdio : [ 'pipe', 'ignore', 'pipe' ] } as options to execSync, which would hide only the standard output (not stderr) of wrangler <command>?

I would love to contribute if the idea swims well with you!

@JacobMGEvans
Copy link
Contributor Author

JacobMGEvans commented Jul 14, 2023

Hey @JacobMGEvans, super cool to see a TypeScript re-write for wrangler-actions! Regarding #105, what do you think of adding a quiet parameter, which when set to true, passes { stdio : 'pipe' } as options to execSync, which would hide the standard output of wrangler <command>?

I would love to contribute if the idea swims well with you!

Definitely wouldn't be against it. I was trying to make this quieter and cleaner (hence grouping) then before 😃 Probably will do that by default for Secrets too (just in case)

@winstxnhdw
Copy link

Awesome! I have created a PR in #113 that addresses this. Please review it :)

test/index.ts Outdated Show resolved Hide resolved
test/index.ts Outdated Show resolved Hide resolved
test/index.ts Outdated Show resolved Hide resolved
.github/workflows/deploy.yml Outdated Show resolved Hide resolved
.github/workflows/deploy.yml Outdated Show resolved Hide resolved
src/index.ts Outdated Show resolved Hide resolved
src/index.ts Outdated Show resolved Hide resolved
test/public/index.html Outdated Show resolved Hide resolved
action.yml Outdated Show resolved Hide resolved
src/index.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@Cherry Cherry left a comment

Choose a reason for hiding this comment

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

Very much liking this 👀

nit: before merge can we get prettier added and code formatted akin to workers-sdk? https://github.com/cloudflare/workers-sdk/blob/main/.prettierrc

src/index.ts Outdated Show resolved Hide resolved
src/index.ts Outdated Show resolved Hide resolved
@JacobMGEvans
Copy link
Contributor Author

Very much liking this 👀

nit: before merge can we get prettier added and code formatted akin to workers-sdk? https://github.com/cloudflare/workers-sdk/blob/main/.prettierrc

@Cherry
Was already planned, I need to make the packages in Workers-SDK actually published, then going to utilize them.

.github/workflows/deploy.yml Outdated Show resolved Hide resolved
.github/workflows/deploy.yml Outdated Show resolved Hide resolved
.github/workflows/deploy.yml Outdated Show resolved Hide resolved
action.yml Outdated Show resolved Hide resolved
src/index.ts Outdated Show resolved Hide resolved
src/index.ts Outdated Show resolved Hide resolved
src/index.ts Outdated Show resolved Hide resolved
src/index.ts Outdated Show resolved Hide resolved
src/index.ts Outdated Show resolved Hide resolved
src/index.ts Outdated Show resolved Hide resolved
@JacobMGEvans JacobMGEvans requested a review from 1000hz July 31, 2023 17:33
src/index.ts Outdated Show resolved Hide resolved
@JacobMGEvans JacobMGEvans force-pushed the jacobmgevans/typescript-overhaul branch from 6192155 to 089567d Compare August 1, 2023 19:37
.github/workflows/deploy.yml Outdated Show resolved Hide resolved
.github/workflows/deploy.yml Show resolved Hide resolved
CHANGELOG.md Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
src/index.ts Show resolved Hide resolved
src/index.ts Show resolved Hide resolved
src/index.ts Show resolved Hide resolved
src/index.ts Outdated Show resolved Hide resolved
test/base/wrangler.toml Show resolved Hide resolved
@JacobMGEvans JacobMGEvans force-pushed the jacobmgevans/typescript-overhaul branch 4 times, most recently from 3f9b0f3 to 6e97c1c Compare August 2, 2023 17:27
action.yml Outdated Show resolved Hide resolved
src/index.ts Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated

### Breaking changes

- Docker is no longer a dependency.
Copy link
Contributor

@1000hz 1000hz Aug 2, 2023

Choose a reason for hiding this comment

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

  • Docker is no longer a dependency.

This isn't a breaking change

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wasn't 100% certain but I will take your word on it lol

CHANGELOG.md Show resolved Hide resolved
CHANGELOG.md Show resolved Hide resolved
@NJordan72
Copy link

After having upgraded I’m getting the following error.

Error: File not found: '/opt/actions-runner/_work/_actions/cloudflare/wrangler-action/3.0.0/dist/index.js'

We are using custom runners. Is there an additional step needed that I’m missing?

@JacobMGEvans JacobMGEvans force-pushed the jacobmgevans/typescript-overhaul branch 4 times, most recently from 4d72254 to ff04a40 Compare August 4, 2023 22:06
@JacobMGEvans
Copy link
Contributor Author

JacobMGEvans commented Aug 4, 2023

After having upgraded I’m getting the following error.

Error: File not found: '/opt/actions-runner/_work/_actions/cloudflare/wrangler-action/3.0.0/dist/index.js'

We are using custom runners. Is there an additional step needed that I’m missing?

@NJordan72 Well the PR is not merged but with this new setup it uses Node for the runner in GH Actions.

@JacobMGEvans JacobMGEvans force-pushed the jacobmgevans/typescript-overhaul branch from ff04a40 to caf6976 Compare August 7, 2023 17:34
Copy link
Contributor

@1000hz 1000hz left a comment

Choose a reason for hiding this comment

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

Good work @JacobMGEvans !

@JacobMGEvans JacobMGEvans changed the title TypeScript Overhaul v4 w/ TypeScript Overhaul Aug 7, 2023
@JacobMGEvans JacobMGEvans force-pushed the jacobmgevans/typescript-overhaul branch from caf6976 to 96a6683 Compare August 7, 2023 18:04
* Removes dependencies such as Docker, decreasing spin-up time
* Adds community-requested features, including bulk secrets API utilization from Wrangler
* Fixes CI/CD
* Adds testing
* Improves command implementation
* Begins using Node for the Action engine/runner
* Openly discusses all changes with the community
  GitHub Discussions opened and Issues monitored

BREAKING CHANGES:
* Docker is no longer a dependency
* Wrangler v1 is no longer supported

Additional related Internal tickets:
Major Version Default: https://jira.cfdata.org/browse/DEVX-632
Rewrite Project: DEVX-804,802,800,632
@JacobMGEvans JacobMGEvans force-pushed the jacobmgevans/typescript-overhaul branch from 96a6683 to edb2a58 Compare August 7, 2023 18:05
@JacobMGEvans JacobMGEvans merged commit 3dd57ff into main Aug 7, 2023
2 checks passed
@JacobMGEvans JacobMGEvans deleted the jacobmgevans/typescript-overhaul branch August 7, 2023 18:08
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.

7 participants