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.

Sign up for GitHub

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

Jump to bottom

[patch] dev experience / gha updates #1080

Merged
merged 1 commit into from
Nov 6, 2024
Merged

[patch] dev experience / gha updates #1080

merged 1 commit into from
Nov 6, 2024
+221 −680

Conversation

bredmond5
Copy link
Contributor

@bredmond5 bredmond5 commented Nov 5, 2024 •
edited
Loading

Pull Request Template

Description

  • Removes a bunch of extraneous ghas and files
  • Creates a single gha that runs tests, and deploys to staging if you merged to master. Tested by commenting out the if statements
  • makes startDev.js actually work
  • removes all references to the dev/ directory. Just use the dist/
  • makes example.html actually get rendered properly

Fixes # (issue)

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

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

  • Unit test
  • Integration test

JS Budget Check

Please mention the size in kb before abd after this PR

Files Before After
dist/build.js.
dist/build.min.js

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules
  • I have checked my code and corrected any misspellings

Mentions:

List the person or team responsible for reviewing proposed changes.

cc @BranchMetrics/saas-sdk-devs for visibility.

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Nov 5, 2024 •
edited
Loading

Test Results

  1 files  ±0    1 suites  ±0   56s ⏱️ +3s
159 tests ±0  152 ✅ ±0  7 💤 ±0  0 ❌ ±0 
162 runs  ±0  155 ✅ ±0  7 💤 ±0  0 ❌ ±0 

Results for commit 6c80eea. ± Comparison against base commit fa7fed4.

♻️ This comment has been updated with latest results.

@bredmond5 bredmond5 force-pushed the update-build branch 5 times, most recently from 0160713 to 04f4b14 Compare November 5, 2024 22:53
@bredmond5 bredmond5 force-pushed the update-build branch 2 times, most recently from 7fe387c to 53146e3 Compare November 5, 2024 22:56
bredmond5
bredmond5 commented Nov 5, 2024
View reviewed changes
.github/workflows/build-push.yml
- name: Run tests
run: npm run test-report

- name: Test Report
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 strange stuff came from the gatekeep.yml

@bredmond5 bredmond5 force-pushed the update-build branch 4 times, most recently from 7eaf059 to 88cefce Compare November 6, 2024 00:22
@bredmond5 bredmond5 changed the title [patch] fix gha for pushing to stage [patch] dev experience / gha updates Nov 6, 2024
bredmond5
bredmond5 commented Nov 6, 2024
View reviewed changes
.github/workflows/gate-keep.yml 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.

i merged gate-keep and deploy-qa into build-push.yml

bredmond5
bredmond5 commented Nov 6, 2024
View reviewed changes
.github/workflows/test.yml 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.

this was a test gha, it shouldnt have been checked in

bredmond5
bredmond5 commented Nov 6, 2024
View reviewed changes
.gitignore
@@ -37,3 +37,6 @@ dev
test-results.json
.idea/
.direnv/
dev.config
dev.html
example.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.

3 generated files

bredmond5
bredmond5 commented Nov 6, 2024
View reviewed changes
Makefile 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.

remove all the references to dev/ directory

bredmond5
bredmond5 commented Nov 6, 2024
View reviewed changes
beta-release.sh 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.

betas are gone

bredmond5
bredmond5 commented Nov 6, 2024
View reviewed changes
src/6_branch.js
if (!error) {
QrCode['rawBuffer'] = rawBuffer;
QrCode['base64'] = function() {
// First Encode array buffer as UTF-8 String, then Base64 Encode
if (this['rawBuffer']) {
const binaryString = Array.from(new Uint8Array(this.rawBuffer))
const binaryString = Array.from(new Uint8Array(rawBuffer))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

well here was the bug

bredmond5
bredmond5 commented Nov 6, 2024
View reviewed changes
startDev.js 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.

now this script actually spits out a working dev.html, and tells the user to use it. It also builds the websdk so it should always have the latest code

@bredmond5 bredmond5 force-pushed the update-build branch 2 times, most recently from afbfbb6 to 87dd827 Compare November 6, 2024 00:27
@bredmond5
[other] refactor development and qa release process
6c80eea 
1. Make startDev actually work out of the box
2. Remove the dev/ directory and everything related to that in favor of just using the output dist/
3. Write proper example.html that is usable in s3 and locally
mack-branch
mack-branch reviewed Nov 6, 2024
View reviewed changes
.github/workflows/build-push.yml
node-version: ${{ env.NODE_VERSION }}
cache: 'npm'

- name: Set up JDK 11
Copy link

Choose a reason for hiding this comment

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

where do we use java?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

im not sure why we are setting up java, i was worried about removing it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ill leave for now, but take an action item to look into it

mack-branch
mack-branch approved these changes Nov 6, 2024
View reviewed changes
Copy link

@mack-branch mack-branch left a comment

Choose a reason for hiding this comment

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

lgtm

@bredmond5 bredmond5 merged commit d78dff0 into master Nov 6, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mack-branch mack-branch mack-branch approved these changes

@gdeluna-branch gdeluna-branch Awaiting requested review from gdeluna-branch

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@bredmond5 @mack-branch