-
Notifications
You must be signed in to change notification settings - Fork 100
[other] ENGMT-1857: add gha for release #1083
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
Conversation
| aws s3 cp example.html s3://branch-cdn/example-staging.html | ||
|
|
||
| ./deployment/build-example-html.sh "key_live_hcnegAumkH7Kv18M8AOHhfgiohpXq5tB" "https://api2.branch.io" "https://cdn.branch.io/branch-latest.min.js" | ||
| aws s3 cp example.html s3://branch-builds/web-sdk/example.html |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
was looking at this more, we did this before my previous pr so i kept it in that pr. But, i dont understand the point of this so i removed it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how is the example page going to be uploaded in this new flow?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks like it happens in a later file
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah this was deploying to prod, and also under this web-sdk prefix for some reason
| check_git_branch | ||
|
|
||
| # update to the latest | ||
| git pull origin master |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
weird, removed
| if [[ $REPLY =~ ^[Yy]$ ]] | ||
| then | ||
| vi CHANGELOG.md | ||
| git commit -am "Updated CHANGELOG.md" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we said we want to have the changelog be manual, so removed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
isn't this manual?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i mean manual as in going into the file and making the edits yourself, then committing
|
|
||
| echo "Building files" | ||
|
|
||
| read -p "Update 0_config.js? " -n 1 -r |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i made this happen by default
| sed -i -e "s/version = '.*';$/version = '$VERSION_NO_V';/" test/web-config.js | ||
| fi | ||
|
|
||
| read -p "Update package.json? " -n 1 -r |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
happens earlier in the gha, before npm ci. I think we need to do that because we need the package.json updated before we run npm ci
| sed -i -e "s/## \[VERSION\] - unreleased/## [$VERSION] - $DATE/" CHANGELOG.md | ||
| fi | ||
|
|
||
| read -p "Update bower.json? " -n 1 -r |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this file seems not used anymore. I left it for now in case removing it blows some things up, but we no longer have an option to write to it
| fi | ||
|
|
||
| echo "Done script." | ||
| read -p "Have you updated the javascript version in https://github.com/BranchMetrics/documentation/edit/master/ingredients/web_sdk/_initialization.md ?" -n 1 -r |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is a dead link
| fi | ||
|
|
||
| echo "Last step, run:" | ||
| echo " git push; git push origin $VERSION" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we're no longer committing the version changes
src/0_config.js
Outdated
| config.link_service_endpoint = 'https://bnc.lt'; | ||
| config.api_endpoint = DEFAULT_API_ENDPOINT; | ||
| config.version = '2.84.0'; | ||
| config.version = '2.85.1'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i checked this in, but technically it is not necessary anymore since the release updates the files before publishing
| fi | ||
| echo -en "Invalidating cloudfront distribution...\n" | ||
| aws configure set preview.cloudfront true | ||
| aws cloudfront create-invalidation --distribution-id E10P37NG0GMER --paths /branch-latest.min.js |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i added cache invalidation
dist/build.js
Outdated
| config.link_service_endpoint = "https://bnc.lt"; | ||
| config.api_endpoint = DEFAULT_API_ENDPOINT; | ||
| config.version = "2.84.0"; | ||
| config.version = "2.85.1"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this supposed to be updated as part of this PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i added a pr comment about this above
| { | ||
| "name": "branch-sdk", | ||
| "version": "2.85.1", | ||
| "version": "0.0.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what's the point of zeroing it out?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
since we are no longer going to be checking in updates to this file, it makes it clear that the version numbers that we publish come from somewhere else
3453cb9 to
f770ade
Compare
Uses the github release as the source of truth. Updates the package.json and various configs with the proper version. This means that the new versions will not be checked into the repo, but they will be deployed properly. I am not sure if that is problematic
.github/workflows/deploy-release.yml
Outdated
| ./deployment/release.sh ${{ steps.next-version.outputs.result }} | ||
| # We deploy to stage to send the updated release numbers. The code should not be any different | ||
| - name: Release to stage |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wouldnt we want to do this first? before prod?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sure
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i mean like as a separate step
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
kind of like how we deploy applications to staging, then verify, then manually release to prod
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could we stage the gha release and new version, and then have a manual action to put this on the prod cdn? does that make sense for something like this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i didnt want to increment the version every time in staging, just increment when we actually deploy. However, that would be another solutoin i suppose
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
my worry would be something like: adds major commit -> updates major version. Finds bug, tags commit major again because we had to change some other part of the api. Now, when we deploy to prod we have incremented 2 versions when we prob should have just done 1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For manual pre-production "publications" we use alpha and beta suffixes. Example being https://www.npmjs.com/package/react-native-branch/v/6.4.0-alpha.0 Incrementing only the last part, without messing with the eventual version (as a suggestion)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah gotcha, sorry, was unaware we were already deploying to staging separately. what youve got looks good, unless maybe we can use the beta/alpha tags like gabe is indicating?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that is an interesting idea, but i think i came up with a better solution
I am just always going to calculate the next version in staging and deploy that. I.e:
current: 2.84, next 2.85
every staging deploy, we check, get 2.84, and deploy 2.85. The github release wont change tho!
on prod, we deploy 2.85, and write 2.85 to releaeses.
Then, next stage deploy we will deploy 2.86 over and over... and on and on
mack-branch
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
one question, otherwise lgtm.
Pull Request Template
Description
Uses the github release version as the source of truth, this is what we do in our other repositories. Updates the package.json and various configs with the proper version.
This means that the new versions will not be checked into the repo, but they will be deployed properly. I am not sure if that is problematic
Removes checking in the build.js and build.min.js in favor of always having it built at runtime then deployed
Fixes # (issue)
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
JS Budget Check
Please mention the size in kb before abd after this PR
Checklist:
Mentions:
List the person or team responsible for reviewing proposed changes.
cc @BranchMetrics/saas-sdk-devs for visibility.