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

RFC: Simplify the workflow for dealing with external assets #5454

Closed
wants to merge 1 commit into from

Conversation

Martchus
Copy link
Contributor

@Martchus Martchus commented Feb 6, 2024

  • Add all external assets directly to our Git repository so no additional downloads are necessary when starting the web application for the first time
  • Adjust .gitignore accordingly
  • Remove/update relevant documentation
  • Clarify licensing of our code vs. third-party code in the README
  • WIP: Add licenses for third-party code now present in our repository
    • Specify the files the LICENSE applies to at the beginning of those files
    • Add MIT licenses individually because each MIT license is technically a distinct license
    • SO FAR ONLY TWO LICENSES HAVE BEEN ADDED; this should be enough for demonstrational purposes
  • See https://progress.opensuse.org/issues/153427
  • See Install assets via npm where possible #5463 for an alternative using npm

@Martchus
Copy link
Contributor Author

Martchus commented Feb 6, 2024

The way I added the third-party licenses (using a Files:-section at the beginning of those files) is like it had been done before for Syncthing Tray. There the Debian maintainer added the licenses this way. So I assume it is the (or at least one) correct way of doing it.

@okurz As you suggested I just used the main branch here. The advantage is indeed that we don't have to do any extra work. No pipelines are required to update anything as one would simply add/update the files in Git as part of the PR that needs the change.

I guess the only downside of this approach is that we add almost 100 000 lines of code. That is 6.4 MiB (without Git overhead). Of course one would need to download these files one way or another so perhaps this isn't a big deal.

On the upside we could now easily track code changes in thrid-party code and never run into download issues when setting up a development environment or when updating the asset cache for the rpm package (which we now would not have to care about at all).

@okurz okurz added the not-ready label Feb 6, 2024
@okurz
Copy link
Member

okurz commented Feb 6, 2024

Seeing this PR I wonder about two points:

  1. So when starting openQA from git then the assets are downloaded automatically?
  2. Can we use npm to at least cover some parts of the asset management and then only store the non-npm controlled JavaScript assets manually directly in git?

docs/Contributing.asciidoc Outdated Show resolved Hide resolved
docs/Contributing.asciidoc Outdated Show resolved Hide resolved
@Martchus
Copy link
Contributor Author

Martchus commented Feb 6, 2024

So when starting openQA from git then the assets are downloaded automatically?

With this change asset pack will find all assets already in place and thus skip the download.

Can we use npm to at least cover some parts of the asset management and then only store the non-npm controlled JavaScript assets manually directly in git?

Maybe that's a good idea, indeed. Of course npm will not put files where we need them but a simple script could copy files to the right location. Or we just specify a local path in asset pack.

* Add all external assets directly to our Git repository so no additional
  downloads are necessary when starting the web application for the first
  time
* Adjust `.gitignore` accordingly
* Remove/update relevant documentation
* Clarify licensing of our code vs. third-party code in the README
* WIP: Add licenses for third-party code now present in our repository
    * Specify the files the LICENSE applies to at the beginning of those
      files
    * Add MIT licenses individually because each MIT license is technically
      a distinct license
    * SO FAR ONLY TWO LICENSES HAVE BEEN ADDED; this should be enough for
      demonstrational purposes
* See https://progress.opensuse.org/issues/153427
@Martchus
Copy link
Contributor Author

Closing in favor of #5463 which is more promising.

@Martchus Martchus closed this Feb 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants