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

Provide VERSION file with asset bundle, read in application #3148

Merged
merged 8 commits into from
Feb 2, 2024

Conversation

grahamalama
Copy link
Contributor

This PR adds a script to generate a VERSION file that gets packaged with our built asset bundle. This is so that applications can read that file to report the Kinto Admin version in a Kinto capabilities object

> npm run build
> tree build -L 1
build
├── VERSION
├── assets
└── index.html

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ugh, forgot about Windows compatibility. I'll port this to Javascript

Copy link
Contributor

Choose a reason for hiding this comment

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

I suppose requiring WSL to build a UI is too much.

bin/generate-version.sh Outdated Show resolved Hide resolved
@@ -4,16 +4,17 @@
"description": "Kinto Web Administration Console in React.js",
"type": "module",
"scripts": {
"build": "vite build",
"build": "npm run generate-version && vite build",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This seems like a job for a Makefile, which can come in another PR. It'd be nice to add our common targets like build, start, lint, format, etc. while we're at it.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think I've ever seen a Makefile for a pure react project before. I'm not opposed to this, but it might cause friction with pure UI devs. We should ask around a bit.

public/.gitkeep Outdated
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If/when we move a "real" file into here, we can ditch this .gitkeep file

import { writeFileSync } from "fs";
import * as https from "https";

async function getLatestRelease() {
Copy link
Contributor

@alexcottner alexcottner Feb 1, 2024

Choose a reason for hiding this comment

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

I think this would be the more "node way" of doing things. But honestly I'm never a fan of just following established patterns, I always prefer what is most readable unless there is a performance issue with that.

#!/usr/bin/env node
import { execSync } from "child_process";
import { writeFileSync } from "fs";
import fetch from "node-fetch"; // i think fetch is the most common package to use, axios is good too

async function getLatestRelease() {
  const res = await fetch("https://api.github.com/repos/Kinto/kinto-admin/releases/latest");
  const body = await res.json();
  return body.tag_name;
}

function getGitLatestReleaseVersion() {
  try {
    return execSync("git describe --tags --abbrev=4", {
      encoding: "utf-8",
    }).trim();
  } catch (error) {
    console.log(error);
  }
};

(async() => {
  const kintoAdminVersion = process.env.KINTO_ADMIN_VERSION // coalesce until we get something
    || getGitLatestReleaseVersion() 
    || await getLatestRelease();
  writeFileSync("./public/VERSION", kintoAdminVersion);
})();

Note: This requires the node-fetch package as a dev dependency.

Copy link
Contributor Author

@grahamalama grahamalama Feb 1, 2024

Choose a reason for hiding this comment

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

Note: This requires the node-fetch package as a dev dependency.

At first I was trying to rely only on the standard library, but since people need to node ci before running node run build anyway, what's one more dependency?

Copy link
Contributor

Choose a reason for hiding this comment

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

I try to keep dependencies slim, but when it comes to things like http clients or database clients I think they're worth it.

@grahamalama grahamalama merged commit 8400176 into master Feb 2, 2024
6 checks passed
@grahamalama grahamalama deleted the provide-version-in-asset-bundle branch February 2, 2024 14:16
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.

3 participants