-
Notifications
You must be signed in to change notification settings - Fork 857
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
Follow up for: Use the umbrella crate for the parachain template #5993 #7464
Open
seemantaggarwal
wants to merge
55
commits into
master
Choose a base branch
from
seemant-parachain-umbrella-crate-mig
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+42
−25
Open
Changes from 46 commits
Commits
Show all changes
55 commits
Select commit
Hold shift + click to select a range
458f1d0
removing unused imports
seemantaggarwal a7eafd4
cargo lock commit
seemantaggarwal 1b4dadf
removing sc-tracing from parachain node
seemantaggarwal 31c7872
restoring dependencies
seemantaggarwal 5a107ef
Merge branch 'master' into seemant-parachain-umbrella-crate-mig
seemantaggarwal 60a91c2
adding sc-tracing in umbrella
seemantaggarwal 64a49ad
importing sc-tracing
seemantaggarwal 397a818
using sc-tracing from polkadot sdk
seemantaggarwal a889bbc
adding sc-tracing-proc-macro
seemantaggarwal 9ae57ca
updating cargo toml for umbrella
seemantaggarwal b2c016a
Update umbrella/Cargo.toml
seemantaggarwal e15c49d
removing breaking umbrella updates
seemantaggarwal ec70c2e
trial and error
seemantaggarwal 8a09b44
creating a new feature for umbrella
seemantaggarwal 38c3dcc
Merge branch 'master' into seemant-parachain-umbrella-crate-mig
seemantaggarwal a62bced
cargo fmt
seemantaggarwal afcd41f
adding additional crate_name resolution for sc-tracing
seemantaggarwal db8ca90
fast forwarding Cargo.lock
seemantaggarwal e20a4f1
adding prdoc
seemantaggarwal 718604e
Merge branch 'master' into seemant-parachain-umbrella-crate-mig
seemantaggarwal ab2822e
Merge branch 'master' into seemant-parachain-umbrella-crate-mig
seemantaggarwal 8310ca2
taplo and fmt
seemantaggarwal 3ef39b8
cargo.lock patch
seemantaggarwal 0ae2a6c
cargo fmt
seemantaggarwal 02d57d5
taplo format
seemantaggarwal 1c69a34
simplifying crate_name code
seemantaggarwal f0f65c8
Merge branch 'master' into seemant-parachain-umbrella-crate-mig
seemantaggarwal 3f36de3
step towards replicating codec into sc-tracing
seemantaggarwal ad6e74a
effort for including sc-tracing in polkadot-sdk
seemantaggarwal 4ad0f0a
changing logic from startswith to fix value for polkadot-sdk resolution
seemantaggarwal 2f95de5
reverting last commit
seemantaggarwal c7a07b9
cargo fmt
seemantaggarwal ac9c846
Merge branch 'master' into seemant-parachain-umbrella-crate-mig
seemantaggarwal 5166954
making further changes
seemantaggarwal e2f570d
further effort, no success
seemantaggarwal 52e0448
implementing suggested changes
seemantaggarwal 13142f1
cargo and taplo
seemantaggarwal 41f6258
resolving cargo.lock conflicts
seemantaggarwal 7cf4e92
Merge branch 'master' into seemant-parachain-umbrella-crate-mig
seemantaggarwal 41a860e
cargo fmt
seemantaggarwal 60f3a6a
adding sc_tracing::loggin in sc-tracing feature
seemantaggarwal 1ed9c36
removing unused imports
seemantaggarwal c93ad26
fixing umbrella crate changes
seemantaggarwal 5391802
reverting changes from Cargo.toml
seemantaggarwal 00c3bfa
explicit import for sc_tracing::logging
seemantaggarwal f0128f5
fixes
serban300 38d5c3c
messy attempt to re‑exports the logging module so that it’s availabl…
seemantaggarwal 545d223
Update from seemantaggarwal running command 'fmt'
github-actions[bot] 3cf488b
adding licenses
seemantaggarwal 8f1997e
Merge branch 'master' into seemant-parachain-umbrella-crate-mig
seemantaggarwal 10b5b92
conflict resolution on Cargo.lock
seemantaggarwal abf5bf5
fix errors
iulianbarbu 51818d8
addressing comments
seemantaggarwal 98a0fb2
Merge branch 'master' into seemant-parachain-umbrella-crate-mig
seemantaggarwal 47e892d
Update from github-actions[bot] running command 'fmt'
github-actions[bot] File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
title: 'Enable importing sc-tracing macros through polkadot-sdk' | ||
doc: | ||
- audience: Node Dev | ||
description: |- | ||
This PR makes it possible to use the sc-tracing macros when they are imported through the umbrella crate. | ||
|
||
crates: | ||
- name: parachain-template-node | ||
bump: minor |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
maybe we can document this somewhere? what this is?
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.
There is a comment above the function definition:
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.
This functionality seems to be shared between frame and substrate client. I would not pull in
frame
namespaced crates as dependencies forsubstrate/client
crates (FYI, a quick fuzzy search forframe
insubstrate/client
is not referencing anyframe
dependency). I wonder if we should add this (and maybe other related logic) to an existing shared place or a new one - e.g.substrate/utils
(although, there isn't a crate for something like it, and we'd need to create one).Also, the function docs miss to mention that it looks for
polkadot-sdk
as well.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.
Yes, also thought of that, but I don't know. I didn't completely check, but I guess
frame-support-procedural-tools
should contain pretty generic methods. If it does, I think we could just use it. Maybe we should rename it or move it underprimitives
. I have to check.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.
@ggwpez @kianenigma what do you think ?
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.
we shouldn't bring in
frame_support_procedural_tools
but rather create a helper function, somewhere accessible to the tracing proc macro, that similarly resolves dependencies.it will check if
polkadot-sdk
is in scope, use that, else look for the crate name directly.It should not care about
polkadot-sdk-frame
, because this is all client side code, which is why I am saying we should not useframe_support_procedural_tools
(other than it being a weird dependency).