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

build: Improve release build & process instructions #540

Merged
merged 6 commits into from
May 7, 2021

Conversation

emostov
Copy link
Contributor

@emostov emostov commented May 7, 2021

Changes:

  • This makes sure we only ship the JS target by adding the "files" key in package.json with just the build folder. There was some regression in bundling where the distribution went from ~9mb => ~90mb. This brings it back down to ~9mb.
  • Update yarn deploy to reflect that we do not automate changelong generation and prevent the footgun of forgetting to build right before publishing.
  • Update README instructions

package.json Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@TarikGul
Copy link
Member

TarikGul commented May 7, 2021

How do some of the suggestions at the bottom of PR #529 which is mentioned in issue #534 sound to you? @emostov

README.md Outdated Show resolved Hide resolved
emostov and others added 2 commits May 6, 2021 18:55
Co-authored-by: Tarik Gul <47201679+TarikGul@users.noreply.github.com>
Co-authored-by: Tarik Gul <47201679+TarikGul@users.noreply.github.com>
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
Copy link
Member

@TarikGul TarikGul left a comment

Choose a reason for hiding this comment

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

LGTM Per my comment above i added a bunch of the suggestion's from that previous PR that were put after we merged.

There is one more suggestion to change the version's to vMM-mm-pp. I think it is a good addition but then could also be confusing.

closes: #534

Co-authored-by: Tarik Gul <47201679+TarikGul@users.noreply.github.com>
@emostov
Copy link
Contributor Author

emostov commented May 7, 2021

There is one more suggestion to change the version's to vMM-mm-pp. I think it is a good addition but then could also be confusing.

Can you expand on that? We already follow semver, so is the suggestion to used - instead of . somewhere?

@TarikGul
Copy link
Member

TarikGul commented May 7, 2021

Yea ofcourse, the suggestion was to replace the use of v5.0.1 as an example and replace it with vMM-mm-pp.

Sorry the was confusing. Basically just replacing our numbers in all the examples above, and replace them with the semver letters.

Copy link
Member

@niklasad1 niklasad1 left a comment

Choose a reason for hiding this comment

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

LGTM, I think it was an excellent battle test of the README, letting me doing the previous release.

Maybe increased bundle size was the reason that the 2FA complained (timeout) ^^

@emostov emostov merged commit ea5b40e into master May 7, 2021
@emostov emostov deleted the zeke-fix-release branch May 7, 2021 14:58
This pull request was closed.
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