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

Set a flag for dev builds to fix manifest issue #149

Merged
merged 3 commits into from
May 23, 2024

Conversation

wojtekn
Copy link
Contributor

@wojtekn wojtekn commented May 22, 2024

Related to https://github.com/Automattic/dotcom-forge/issues/7339

Proposed Changes

I propose to set an environment variable to state that the given build is a dev build. That way, the manifest can be generated correctly for dev builds after recent pipeline changes.

Testing Instructions

See if the manifest is generated correctly and if dev builds are updated.

Currently, 'Distribute Dev Builds' step runs only for trunk, so it can't be tested before the merge.

Pre-merge Checklist

  • Have you checked for TypeScript, React or other console errors?

@wojtekn wojtekn self-assigned this May 22, 2024
@wojtekn wojtekn requested a review from mokagio May 22, 2024 06:34
Copy link
Member

@p-jackson p-jackson left a comment

Choose a reason for hiding this comment

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

LGTM.

The step doesn't run on PR builds, so there's no better way to test other than to merge. YOLO.

@@ -71,6 +71,7 @@ steps:
buildkite-agent artifact download "*.exe" .

echo "--- :node: Generating Release Manifest"
export IS_DEV_BUILD=true
Copy link
Member

Choose a reason for hiding this comment

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

Or IS_DEV_BUILD=true node ./scripts/generate-releases-manifest.mjs to make it explicit the flag is specifically for the generate-releases-manifest script.

@wojtekn wojtekn merged commit 1fe21c8 into trunk May 23, 2024
11 checks passed
@wojtekn wojtekn deleted the update/generating-dev-release-manifest branch May 23, 2024 07:59
@wojtekn
Copy link
Contributor Author

wojtekn commented May 23, 2024

It worked fine, as the 'dev' builds section in manifest was regenerated:

Version: 1.0.2
--
  | Is dev build: true
  | Current commit: 1fe21c8
  | Downloading current manifest ...
  | (node:1670) ExperimentalWarning: Importing JSON modules is an experimental feature and might change at any time
  |  
  | Download complete
  | Parsing current release info ...
  | Overriding latest dev release ...
  | Overwrote latest dev release
  | Done generating releases manifest.

And here is the section:

% curl https://cdn.a8c-ci.services/studio/releases.json
{
  "dev": {
    "darwin": {
      "universal": {
        "sha": "1fe21c8",
        "url": "https://cdn.a8c-ci.services/studio/studio-darwin-1fe21c8.app.zip"
      },
      "x64": {
        "sha": "1fe21c8",
        "url": "https://cdn.a8c-ci.services/studio/studio-darwin-x64-1fe21c8.app.zip"
      },
      "arm64": {
        "sha": "1fe21c8",
        "url": "https://cdn.a8c-ci.services/studio/studio-darwin-arm64-1fe21c8.app.zip"
      }
    },
    "win32": {
      "sha": "1fe21c8",
      "url": "https://cdn.a8c-ci.services/studio/studio-win32-1fe21c8.exe"
    }
  },
[...]

However, links don't work so we need to check it more.

@p-jackson
Copy link
Member

I notice the -universal- bit is missing from the universal build's filename. But I don't know what's wrong with the other ones. Should the filename include -v1.0.2-dev.1fe21c8?

@wojtekn
Copy link
Contributor Author

wojtekn commented May 23, 2024

@p-jackson I fixed those in #153

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