Skip to content
This repository was archived by the owner on Jun 21, 2024. It is now read-only.

Comments

Remaining work on packaging for publish.#11

Merged
chrisdevereux merged 22 commits intomainfrom
publish-to-package-managers
Dec 9, 2021
Merged

Remaining work on packaging for publish.#11
chrisdevereux merged 22 commits intomainfrom
publish-to-package-managers

Conversation

@chrisdevereux
Copy link
Contributor

@chrisdevereux chrisdevereux commented Dec 3, 2021

Description

This PR:

  • Adds CI hooks to build & publish to npm/pypi when a GitHub release is created.
  • Pre-builds static assets into the pypi package before release (usable without a frontend build toolchain).
  • Builds an esmodule version for npm (usable by frontend bundlers).
  • Builds d.ts files for npm
  • Drop typescript path aliases because they were interfering with d.ts file generation
  • Switch out ts-jest for babel-jest because it didn't play nicely with some newer js features vite supports (import.meta)
  • Renames pyck -> groundwork
  • Adds a template tag for django to load assets in environments without a frontend build toolchain.

Motivation and Context

This library needs publishing before people can use it!

Was unsure about whether to tackle this now or defer it, but it'd be quite annoying to use without doing so – the npm package needs building before install and in practice so does the python package, as it includes a bunch of static js and css.

How Can It Be Tested?

  • Test deployment to npm
  • Test deployment to pypi

Hard to fully test yourself because it involves a release hook on CI. I have done by temporarily switching the CI trigger to pull-request and running it on this.

Types of changes

  • Tooling change

Checklist:

  • I have documented all new methods using google docstring format.
  • I have added type annotations to any public API methods.
  • I have updated any relevant high-level documentation.
  • I have added a usage example to the example app if relevant.
  • I have written tests covering all new changes.

@chrisdevereux chrisdevereux force-pushed the publish-to-package-managers branch from a03785b to 35f4570 Compare December 3, 2021 16:46
@conatus
Copy link
Member

conatus commented Dec 6, 2021

@chrisdevereux

Do you want this reviewed now? I am presuming you do not right this moment, but do request my review when you want it looked at.

@chrisdevereux chrisdevereux force-pushed the publish-to-package-managers branch from 35f4570 to 6fc227f Compare December 7, 2021 19:52
@chrisdevereux chrisdevereux force-pushed the publish-to-package-managers branch from 6fc227f to b1723cf Compare December 9, 2021 14:30
@chrisdevereux chrisdevereux marked this pull request as ready for review December 9, 2021 16:25
@chrisdevereux
Copy link
Contributor Author

chrisdevereux commented Dec 9, 2021

@conatus Sorry, missed that. No, I did not. Now I do!

Copy link
Member

@janbaykara janbaykara left a comment

Choose a reason for hiding this comment

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

Not seeing anything wildly wrong here.

Occurred to me that there could be scope for including a django variant of Ruby-style import maps, for loading Stimulus controller files for templates, as an alternative to bundling them altogether.

Glad to see this publish soon!

@@ -0,0 +1,8 @@
Groundwork is distributed as a Python library [via PyPI](https://pypi.org/project/groundwork-django/). Frontend components are distibuted [via NPM](https://npmjs.org/package/groundwork-ui).
Copy link
Member

@janbaykara janbaykara Dec 9, 2021

Choose a reason for hiding this comment

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

Maybe repeat the package slug groundwork-django here, for easy copy pasta? This could also be echoed in README.md. I note that groundwork is actually taken by another package, but that would have been my immediate guess.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good call. Updated.

@chrisdevereux
Copy link
Contributor Author

chrisdevereux commented Dec 9, 2021

Not seeing anything wildly wrong here.

Occurred to me that there could be scope for including a django variant of Ruby-style import maps, for loading Stimulus controller files for templates, as an alternative to bundling them altogether.

We in fact kind of already have this behaviour by using dynamic import() to load and register controllers on-demand

This is also exposed as a utility to the application, which it can use to load its own controllers, and does by default in the starter template. It should be better documented though! In general, I haven't written anything on the stimulus side of the library. I'll make a task to do that.

Using import maps could prove to be a more 'standard' way of providing this behaviour when the django hotwired libraries mature a bit. This is nice in that it doesn't impose any constraints on how the django app is configured though.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants