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

Upload windows stub/standalone installers to Github. #810

Merged
merged 1 commit into from Nov 6, 2018
Merged

Conversation

@mbacchi
Copy link
Member

mbacchi commented Nov 6, 2018

Also update DEPS to use the most recent changes that
build and sign the installers during the
create_dist --build_omaha step.

Fixes brave/brave-browser#1992

Submitter Checklist:

  • Submitted a ticket for my issue if one did not already exist.
  • Used Github auto-closing keywords in the commit message.
  • Added/updated tests for this change (for new code or code which already has tests).
  • Verified that these changes build without errors on
    • Windows
    • macOS
    • Linux
  • Verified that these changes pass automated tests (npm test brave_unit_tests && npm test brave_browser_tests) on
    • Windows
    • macOS
    • Linux
  • Ran git rebase master (if needed).
  • Ran git rebase -i to squash commits (if needed).
  • Tagged reviewers and labelled the pull request as needed.
  • Request a security/privacy review as needed.
  • Add appropriate QA labels (QA/Yes or QA/No) to include the closed issue in milestone

Test Plan:

Reviewer Checklist:

  • New files have MPL-2.0 license header.
  • Request a security/privacy review as needed.
  • Adequate test coverage exists to prevent regressions
  • Verify test plan is specified in PR before merging to source
Also update DEPS to use the most recent changes that
build and sign the installers during the
`create_dist --build_omaha` step.

Fixes brave/brave-browser#1992
@mbacchi mbacchi self-assigned this Nov 6, 2018
@mbacchi mbacchi requested review from mihaiplesa, bsclifton and simonhong Nov 6, 2018
@simonhong
Copy link
Collaborator

simonhong commented Nov 6, 2018

How about appending version to uploaded brave_installer.exe and standalone installer? @bbondy

@bbondy
Copy link
Member

bbondy commented Nov 6, 2018

I think it might be easier for some automation and such to not have the version in the filename, but having the version in the file properties should be there.

@simonhong
Copy link
Collaborator

simonhong commented Nov 6, 2018

I checked that brave_installer.exe alreay has proper version info in file properties.
But, standalone installer has omaha version in file properties.
Created issue for that. brave/brave-browser#2002

@simonhong
Copy link
Collaborator

simonhong commented Nov 6, 2018

I'll pass to @mihaiplesa for reviewing (I don't know much about uploading stuff)

@bsclifton bsclifton mentioned this pull request Nov 6, 2018
3 of 3 tasks complete
Copy link
Collaborator

mihaiplesa left a comment

looks good, can remove some code duplication by having a variable named suffix that takes values of '' or '32' based on arch

@mbacchi mbacchi merged commit cfa1807 into master Nov 6, 2018
1 check failed
1 check failed
continuous-integration/travis-ci/pr The Travis CI build failed
Details
@mihaiplesa mihaiplesa deleted the issue_1992 branch Nov 6, 2018
@kjozwiak
Copy link
Member

kjozwiak commented Nov 6, 2018

Approved uplift to 0.56.x 👍

NejcZdovc added a commit that referenced this pull request Nov 6, 2018
Upload windows stub/standalone installers to Github.
NejcZdovc added a commit that referenced this pull request Nov 6, 2018
Upload windows stub/standalone installers to Github.
@NejcZdovc
Copy link
Member

NejcZdovc commented Nov 6, 2018

master (0.59) cfa1807
0.58.x da23ee0
0.57.x c795374
0.56.x some merge conflicts @mbacchi can you please cherry pick it to 56

mbacchi added a commit that referenced this pull request Nov 6, 2018
Upload windows stub/standalone installers to Github.
@mbacchi
Copy link
Member Author

mbacchi commented Nov 6, 2018

0.56.x: 1f389f9 (had to uplift #747 first to avoid merge conflict)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

7 participants
You can’t perform that action at this time.