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 workflow steps for building #13

Merged
merged 3 commits into from
Aug 5, 2024
Merged

Add workflow steps for building #13

merged 3 commits into from
Aug 5, 2024

Conversation

kevin-david
Copy link
Owner

And remove unnecessary action

@kevin-david kevin-david merged commit 4585d16 into main Aug 5, 2024
3 checks passed
@kevin-david kevin-david deleted the build branch August 5, 2024 00:26
@Yakov5776
Copy link

Yakov5776 commented Aug 5, 2024

Why can't we add npm i as a workflow step so we don't need to track node_modules in git history?

@Yakov5776
Copy link

Yakov5776 commented Aug 5, 2024

@kevin-david Also do you plan on bumping the release version? Since there are changes to the workflow inputs.

@kevin-david
Copy link
Owner Author

Why can't we add npm i as a workflow step so we don't need to track node_modules in git history?

That's a good idea given the requirements for this repo, I'd rather not pollute diffs. Fixed in #16

@kevin-david Also do you plan on bumping the release version? Since there are changes to the workflow inputs.

Yeah I'll push out v2 shortly. I want to clean a few things up first

@kevin-david
Copy link
Owner Author

@Yakov5776

@kevin-david Also do you plan on bumping the release version? Since there are changes to the workflow inputs.

https://github.com/kevin-david/zipalign-sign-android-release/releases/tag/v2.0.0 is out - let me know if you see any issues.

@Yakov5776
Copy link

@Yakov5776

@kevin-david Also do you plan on bumping the release version? Since there are changes to the workflow inputs.

https://github.com/kevin-david/zipalign-sign-android-release/releases/tag/v2.0.0 is out - let me know if you see any issues.

Thank you, will take a look at it tomorrow or later tonight.

@Yakov5776
Copy link

Yakov5776 commented Aug 5, 2024

@Yakov5776

@kevin-david Also do you plan on bumping the release version? Since there are changes to the workflow inputs.

https://github.com/kevin-david/zipalign-sign-android-release/releases/tag/v2.0.0 is out - let me know if you see any issues.

Either rename the tag to just v2 or fix the readme to point to v2.0.0 because it's supposed to be an exact tag.
Also you should probably do some testing because right off the bat it fails for resolving a module - not sure why.

edit 1: I'd delete the tag for now, I was under the impression that node_modules wasn't needed since the lib/main.js includes it pre-bundled but that clearly isn't the case and it probably relied on it.

I wouldn't suggest adding it back as there's better solutions out there without tracking node_modules, for example if we add ncc as a second build step that could work.

edit 2: ncc seems to have built-in typescript support so we may even be able to ditch tsc altogether, if i get it working i'll send a patch.

edit 3: i have submitted a patch at #18, take a look.

@kevin-david
Copy link
Owner Author

Also you should probably do some testing because right off the bat it fails for resolving a module - not sure why.

I think #20 will prevent this kind of mistake in the future!

This pull request was closed.
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.

2 participants