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

feat: add platform field to run options to support Apple M1 #344

Merged
merged 2 commits into from Mar 22, 2022

Conversation

tainguyentt
Copy link
Contributor

@tainguyentt tainguyentt commented Mar 8, 2022

Related issue(s)

Resolves #283

Checklist

  • I have read the contributing guidelines
    and signed the CLA.
  • I have read the security policy.
  • I confirm that this pull request does not address a security
    vulnerability. If this pull request addresses a security vulnerability, I
    confirm that I got green light (please contact
    security@ory.sh) from the maintainers to push
    the changes.
  • I have added tests that prove my fix is effective or that my feature
    works.
  • I have added necessary documentation within the code base (if
    appropriate).

Further comments

  • The PR feat: Add platform param to build options #315 only resolved the issue of building docker images on Mac M1, however we couldn't start containers using pre-built images (in my case is MySQL 5.7 docker image)
  • While waiting for this PR to be merged, there is a workaround that we can pull the docker image first before running the tests. E.g.
docker pull --platform linux/amd64 mysql:5.7
go test ./... 

@codecov
Copy link

codecov bot commented Mar 8, 2022

Codecov Report

No coverage uploaded for pull request base (v3@e58dff8). Click here to learn what that means.
The diff coverage is n/a.

Current head 92b0eae differs from pull request most recent head dbdb34c. Consider uploading reports for the commit dbdb34c to get more accurate results

Impacted file tree graph

@@          Coverage Diff          @@
##             v3     #344   +/-   ##
=====================================
  Coverage      ?   53.47%           
=====================================
  Files         ?        1           
  Lines         ?      331           
  Branches      ?        0           
=====================================
  Hits          ?      177           
  Misses        ?      117           
  Partials      ?       37           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e58dff8...dbdb34c. Read the comment docs.

@tainguyentt tainguyentt changed the title Feat: add platform field to run options feat: add platform field to run options Mar 8, 2022
@tainguyentt tainguyentt changed the title feat: add platform field to run options feat: add platform field to run options to support Apple M1 Mar 8, 2022
@aeneasr
Copy link
Member

aeneasr commented Mar 9, 2022

Thank you! Could you add a small test verifying that this feature works? :)

@tainguyentt tainguyentt marked this pull request as draft March 10, 2022 14:07
@tainguyentt tainguyentt marked this pull request as ready for review March 10, 2022 14:19
@tainguyentt tainguyentt reopened this Mar 10, 2022
@tainguyentt
Copy link
Contributor Author

tainguyentt commented Mar 10, 2022

Thank you! Could you add a small test verifying that this feature works? :)

Hi @aeneasr, I have added a test for MySQL. Could you help to review again? Thanks

@aeneasr aeneasr merged commit ef54b32 into ory:v3 Mar 22, 2022
8 checks passed
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.

Feature request: Add support for API version 1.32+ for M1 mac users
2 participants