Skip to content

Conversation

@gonfunko
Copy link
Contributor

@gonfunko gonfunko commented Jul 7, 2021

The basics

  • I branched from develop
  • My pull request is against develop
  • My code follows the style guide

The details

Resolves

#4977

Proposed Changes

Adds a script that calculates the size of the various Blockly JS files and updates check_metadata.sh with the new values.

Behavior Before Change

As part of the release process, a developer would need to manually determine the build artifact sizes and update the check_metadata.sh script.

Behavior After Change

Running the workflow will automatically calculate the new build artifact sizes, update the script, and create a PR with the updated script.

Reason for Changes

Makes this step in the release process more reliable and reduces the effort on the part of the dev team.

dependabot bot and others added 5 commits July 1, 2021 15:58
Bumps [ws](https://github.com/websockets/ws) from 7.4.4 to 7.5.1.
- [Release notes](https://github.com/websockets/ws/releases)
- [Commits](websockets/ws@7.4.4...7.5.1)

---
updated-dependencies:
- dependency-name: ws
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>
Bumps [lodash](https://github.com/lodash/lodash) from 4.17.19 to 4.17.21.
- [Release notes](https://github.com/lodash/lodash/releases)
- [Commits](lodash/lodash@4.17.19...4.17.21)

---
updated-dependencies:
- dependency-name: lodash
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>
Copy link
Collaborator

@cpcallen cpcallen left a comment

Choose a reason for hiding this comment

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

I was going to suggest that you add a flag to check_metadata.sh to do this, rather than creating a separate script, but I've learned that bash acts weirdly in the face of self-modifying shell scripts so you'd want to store the target sizes and history data in a separate file. (There are ways to make self-modifying bash scripts work safely but… yikes.)

Comment on lines +13 to +19
replacement="# ${quarter} ${version} ${blockly_size}\nblockly_size_expected=${blockly_size}"
sed -ri "s/blockly_size_expected=[0-9]+/${replacement}/g" tests/scripts/check_metadata.sh
replacement="# ${quarter} ${version} ${blocks_size}\nblocks_size_expected=${blocks_size}"
sed -ri "s/blocks_size_expected=[0-9]+/${replacement}/g" tests/scripts/check_metadata.sh
replacement="# ${quarter} ${version} ${blockly_gz_size}\nblockly_gz_size_expected=${blockly_gz_size}"
sed -ri "s/blockly_gz_size_expected=[0-9]+/${replacement}/g" tests/scripts/check_metadata.sh
replacement="# ${quarter} ${version} ${blocks_gz_size}\nblocks_gz_size_expected=${blocks_gz_size}"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Note that most of the history lines in check_metadata.sh were tab-delimited (and I've fixed the ones which aren't in PR #4968). If you prefer to change that then I suggest waiting for #4968 then including a commit to reformat the history in this PR.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh, also: see [style guide re: line length and long string literals](replacement="# ${quarter} ${version} ${blockly_size}\nblockly_size_expected=${blockly_size}").

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I noticed the delimiter issue and decided to just go with the most recent, but having it be consistent SGTM, will hold off until that change is in.

rachel-fenichel and others added 13 commits July 8, 2021 19:15
…npm_and_yarn/ws-7.5.1

Bump ws from 7.4.4 to 7.5.1
…npm_and_yarn/lodash-4.17.21

Bump lodash from 4.17.19 to 4.17.21
…pengine_demos_master

Appengine deploy workflow on master for testing
Previous path was `_deploy/`. New path is `../_deploy`.
…chel-patch-1

Get deploy files from the correct directory
…rryPiFoundation#5006)

* Create develop_freeze_comment.yml

* Update comments

* Fix typo and update uses

* Add test message
Bumps [hosted-git-info](https://github.com/npm/hosted-git-info) from 2.8.4 to 2.8.9.
- [Release notes](https://github.com/npm/hosted-git-info/releases)
- [Changelog](https://github.com/npm/hosted-git-info/blob/v2.8.9/CHANGELOG.md)
- [Commits](npm/hosted-git-info@v2.8.4...v2.8.9)

---
updated-dependencies:
- dependency-name: hosted-git-info
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…-rachel-fenichel-patch-1

Revert "Get deploy files from the correct directory"
@gonfunko gonfunko requested a review from a team as a code owner July 12, 2021 19:54
@gonfunko gonfunko requested a review from BeksOmega July 12, 2021 19:54
@gonfunko
Copy link
Contributor Author

I managed to clobber my branch, so I'm going to withdraw this PR and send a new one incorporating the changes you suggested.

@gonfunko gonfunko closed this Jul 12, 2021
@gonfunko gonfunko deleted the update-metadata branch July 12, 2021 20:33
@gonfunko
Copy link
Contributor Author

#5033

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.

4 participants