Merged
Conversation
Using a fork due to paritytech/polkadot-sdk#4344 not being integrated in v1.11.0 yet.
Closed
serg-temchenko
suggested changes
May 11, 2024
Contributor
serg-temchenko
left a comment
serg-temchenko
left a comment
There was a problem hiding this comment.
Just to clarify, are the implementations from the node and runtime folders simply reflections of the template code, or have there been adjustments and changes made beyond the .toml files?
- Could you please reorganize all indents from hard tabs to spaces?
- I noticed that we already have a
.gitignorefile in the main branch. However, I have some questions about it, particularly regarding whether it was taken from some convention definition. Why does it contain many random entries like.AppleDBetc? For example, here is the default .gitignore for Rust projects, and here is one from substrate. My point is simply to add code with an understanding of its requirements. - I notice that we have a .lock file in the gitignore, and I strongly believe that this file should be under version control rather than ignored. This ensures a stable build, as an example, situation you experienced with this task and forks and substrate dependency, which has issues when using the master branch...
- Also, please try to fix that issue with build by overwrite that libp2p dependency version. I believe this might help.
The original polkadot-sdk was built pointing litep2p to master at the time of the build, master was version 0.3.0, commit b142c9eb611fb2fe78d2830266a3675b37299ceb, this patch is required since the dependency points to master and not the version/hash
serg-temchenko
suggested changes
May 14, 2024
Contributor
There was a problem hiding this comment.
Perfect! Thank you @jmg-duarte for resolving comments and fixing the problem with indirect dependencies. Great job! 🔥 One small comment and we are good to go!
Also, please update the description and remove any mention of the fork to avoid confusing people in the future. I've updated it. Please feel free to change it if you have other preferences for how to highlight this.
Merged
th7nder
reviewed
May 14, 2024
4 tasks
serg-temchenko
approved these changes
May 14, 2024
th7nder
approved these changes
May 14, 2024
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Setup the project structure according to the guidelines, namely:
Points to a polkadot-sdk fork due to paritytech/polkadot-sdk#4344Fixed by pinning the working version.Fixes #1