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

Builds with v4.3.0 instead of fork of older version of SQL Cipher [ci full] #2732

Closed
wants to merge 1 commit into from
Closed

Builds with v4.3.0 instead of fork of older version of SQL Cipher [ci full] #2732

wants to merge 1 commit into from

Conversation

dp-mason
Copy link
Contributor

@dp-mason dp-mason commented Feb 28, 2020 •
edited
Loading

Pull Request checklist

  • Quality: This PR builds and tests run cleanly
    • automation/all_tests.sh runs to completion and produces no failures
    • Note: For changes that need extra cross-platform testing, consider adding [ci full] to the PR title.
  • Tests: This PR includes thorough tests or an explanation of why it does not
  • Changelog: This PR includes a changelog entry in CHANGES_UNRELEASED.md or an explanation of why it does not need one
    • Any breaking changes to Swift or Kotlin binding APIs are noted explicitly
  • Dependencies: This PR follows our dependency management guidelines
    • Any new dependencies are accompanied by a summary of the due dilligence applied in selecting them.

@eoger eoger changed the title Builds with v4.3.0 instead of fork of older version of SQL Cipher Builds with v4.3.0 instead of fork of older version of SQL Cipher [ci full] Feb 28, 2020
thomcc
thomcc approved these changes Feb 28, 2020
View reviewed changes
Copy link
Contributor

@thomcc thomcc left a comment

Choose a reason for hiding this comment

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

LGTM assuming CI comes up green, but @eoger would know if there's anything missing.

@thomcc thomcc requested a review from eoger February 28, 2020 21:34
@eoger
Copy link
Contributor

eoger commented Feb 28, 2020

@dp-mason do you mind rebasing against master? There's a conflict in CHANGES_UNRELEASED.md preventing the merge. Also you will need to sign your commits.

@dp-mason
Set LTO to thin. Fixes mozilla#2531. [ci full]
f1c3678 
Build with SQL Cipher 4.3.0 with NSS backend

Build with SQL Cipher 4.3.0
thomcc
thomcc suggested changes Feb 29, 2020
View reviewed changes
CHANGES_UNRELEASED.md
@@ -2,4 +2,24 @@

# Unreleased Changes

<<<<<<< HEAD
Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, looks like you need to resolve this conflict still (Let us know if you need help with this)

@eoger
Copy link
Contributor

eoger commented Mar 1, 2020

Also it appears you rebased on top of @thomcc commit and amended it: "Set LTO to thin. Fixes #2531. [ci full]". I would suggest reading a little bit about git merging and rebasing strategies.

eoger
eoger suggested changes Mar 3, 2020
View reviewed changes
Copy link
Contributor

@eoger eoger left a comment

Choose a reason for hiding this comment

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

Clearing r? for now

@vladikoff vladikoff mentioned this pull request Mar 16, 2020
Merged
4 tasks
@vladikoff
Copy link
Contributor

rebased in #2822

@vladikoff vladikoff closed this Mar 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@eoger eoger eoger requested changes

@thomcc thomcc thomcc requested changes

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

Successfully merging this pull request may close these issues.
4 participants
@dp-mason @eoger @vladikoff @thomcc