-
Notifications
You must be signed in to change notification settings - Fork 174
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(libp2p): Optional async std libp2p & profiling profile #1454
Conversation
c04b27a
to
7b5e71c
Compare
9426802
to
db73537
Compare
db73537
to
f16b088
Compare
.github/workflows/snapshot.yml
Outdated
@@ -59,11 +59,11 @@ jobs: | |||
publish: false | |||
|
|||
- name: Run cargo build | |||
run: cargo build --release -p particle-node | |||
run: cargo build --profile release-with-debug -p particle-node |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure we want that for every snapshot. WDYT?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For dev purposes, it is extremely useful. You can debug and profile these images.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With this approach, we loose non-dev snapshots for the branch.
These snapshots are used in three ways:
- in e2e tests, where these snapshots must be fast
- to deploy temporary fixes to our environments, even to kras, and it must be fast there
- profiling - pretty rare thing
To me, it seems that profile-able images are better to be ran on request. Maybe manually changing profile to profiling
(aka release-with-debug) in CI scripts is a good-enough approach.
If we choose that path, then I'd keep profile in Cargo.toml, but rename it to profiling
.
4764563
to
94a63d5
Compare
No description provided.