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 publish-denoland workflow #64

Closed
wants to merge 8 commits into from
Closed

Conversation

jcbhmr
Copy link
Collaborator

@jcbhmr jcbhmr commented Jun 3, 2023

This PR would...

  • Add a publish workflow for https://deno.land/x that uses a build step to create a deno/5.0.0 tag on the 'build' branch whenever a new release is created!
  • Run npm script 'build:deno' which uses denoify
  • Uses denoify to create a deno_dist folder with a mod.ts file and the README.md and LICENSE preserved
  • Allows the removal of dist from the source tree! 🎉 Actually this was me misunderstanding what deno users were doing

@jcbhmr jcbhmr requested a review from mesqueeb June 3, 2023 19:31
@jcbhmr jcbhmr self-assigned this Jun 3, 2023
@mesqueeb
Copy link
Owner

mesqueeb commented Jun 6, 2023

@jcbhmr I don't understand why we need this? 🤔
Deno already gets auto updated without any additional config or github actions:
https://deno.land/x/is_what

@jcbhmr
Copy link
Collaborator Author

jcbhmr commented Jun 6, 2023

@jcbhmr I don't understand why we need this? 🤔 Deno already gets auto updated without any additional config or github actions: https://deno.land/x/is_what

Because right now deno users are using the /src/index.ts in their code:

image
https://github.com/search?q=https%3A%2F%2Fdeno.land%2Fx%2Fis_what&type=code

This change would publish a denoified version of this package which would allow users to use the /mod.ts export instead. It comes down to this question: Is deno a first-class publishing target like Node.js+npm, or is it a second-class platform?

I think we're leaning towards the second option of second-class as more of a "cool side-effect but not really the focus of this package". Is that right? If so, yes this workflow is redundant.

@jcbhmr jcbhmr marked this pull request as draft June 6, 2023 20:49
@mesqueeb
Copy link
Owner

mesqueeb commented Jun 6, 2023

@jcbhmr No you're right. I didn't realise that is_what needed to be used as per your screenshots in deno.

This workflow causes no harm. I'd love to merge it.

Some things you threw out of the .gitignore that should stay though...? like .DS_Store for macOS.

Not sure we need all the extra crap on next, nuxt, gatsby, vitepress, since we won't use them for is-what. 😅 but ah well, it's just a gitignore file no one looks at

@jcbhmr
Copy link
Collaborator Author

jcbhmr commented Jun 6, 2023

Guess what! There's a well-defined standard workflow for those pesky .DS_Store files for .gitignore! 😊

Global contains templates for various editors, tools and operating systems that can be used in different situations. It is recommended that you add these to your global template.

https://github.com/github/gitignore#readme

@jcbhmr jcbhmr marked this pull request as ready for review June 6, 2023 21:06
@jcbhmr jcbhmr changed the base branch from production to use-tsc June 6, 2023 21:37
@jcbhmr jcbhmr marked this pull request as draft June 6, 2023 21:39
@jcbhmr
Copy link
Collaborator Author

jcbhmr commented Jun 6, 2023

This PR requires the outDir option to be set in tsconfig.json and is therefore blocked by #74

@jcbhmr
Copy link
Collaborator Author

jcbhmr commented Jul 17, 2023

Given that another month has passed, I think that the best way forward is just to say "use the npm: specifier" since it's become much much better over the past few months. You can now deno compile with code that uses npm: and it works with more packages than ever before!

Thus, I think the right way to move forward is to NOT merge this and instead focus on making a good npm package. Deno npm: translation will handle the rest. 😊

@jcbhmr jcbhmr closed this Jul 17, 2023
@jcbhmr jcbhmr deleted the add-publish-denoland branch July 17, 2023 21:22
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