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

Cargo workspace yassin feedback #136

Merged
merged 8 commits into from
Nov 19, 2023

Conversation

YassinEldeeb
Copy link
Member

@YassinEldeeb YassinEldeeb commented Nov 19, 2023

What is applied from @dotansimha's PR?

  • Resolve Actix-web Tail segments must have names. warning
  • Remove strip = false from CI, since it's defaulted to false
  • Rename async_runtime crate to wasm_polyfills
  • Use wasm_polyfills items inline without imports for better clarity.
  • Separate crates/* into bin/* and libs/*
  • Rename from_yaml to parse_config_from_yaml for better clarity, and extract parse_config_from_json as a utility as well + reuse those in the main flow.
  • Remove the unused openssl dependency

@YassinEldeeb
Copy link
Member Author

Agh...rebasing files I never touched is a pain...🥲

@dotansimha
Copy link
Member

@YassinEldeeb why not target my branch? why is it targeted to master?

@dotansimha dotansimha changed the base branch from master to cargo-workspace November 19, 2023 07:05
@dotansimha
Copy link
Member

dotansimha commented Nov 19, 2023

  • Without openssl, the compilation inside Docker image (as part of the Dockerize step) fails...
  • Fix CI and paths for Docker build
  • Fix WASM CI with new paths

Copy link

github-actions bot commented Nov 19, 2023

🐋 This PR was built and pushed to the following Docker images:

Docker Bake metadata
{
"conductor": {
  "containerimage.config.digest": "sha256:8e686644095ed924e63ef55cc02309142af9236b56f1823ecd771ed9302134fd",
  "containerimage.descriptor": {
    "mediaType": "application/vnd.docker.distribution.manifest.v2+json",
    "digest": "sha256:2dda9bea5d1cdd50cdd10624b07a5fc107e9d09dac1d27a8afd74fcd2ebebca6",
    "size": 901,
    "platform": {
      "architecture": "amd64",
      "os": "linux"
    }
  },
  "containerimage.digest": "sha256:2dda9bea5d1cdd50cdd10624b07a5fc107e9d09dac1d27a8afd74fcd2ebebca6",
  "image.name": "ghcr.io/the-guild-org/conductor-t2/conductor:a365171d1bafed7abff120040b7f78cb900d1a47"
}
}

@dotansimha dotansimha merged commit d3ac18f into cargo-workspace Nov 19, 2023
10 checks passed
@dotansimha dotansimha deleted the cargo-workspace-yassin-feedback branch November 19, 2023 08:58
dotansimha added a commit that referenced this pull request Nov 19, 2023
Co-authored-by: Dotan Simha <dotansimha@gmail.com>
dotansimha added a commit that referenced this pull request Nov 19, 2023
Co-authored-by: Yassin Eldeeb <yassineldeeb94@gmail.com>
@YassinEldeeb
Copy link
Member Author

@YassinEldeeb why not target my branch? why is it targeted to master?

oh, that's the reason...I missed it 😄

Thanks for all the work on landing it!! 🚀

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.

None yet

2 participants