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.

Sign up for GitHub

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

Jump to bottom

Improve file structure #590

Open
xlc opened this issue Aug 1, 2023 · 17 comments
Open

Improve file structure #590

xlc opened this issue Aug 1, 2023 · 17 comments
Assignees
@GauravDhak
Labels
C1-mentor A task where a mentor is available. Please indicate in the issue who the mentor could be. C2-good-first-issue A task for a first time contributor to become familiar with the Polkadot-SDK. D0-easy Can be fixed primarily by duplicating and adapting code by an intermediate coder. I4-refactor Code needs refactoring.

Comments

@xlc
Copy link
Contributor

xlc commented Aug 1, 2023

We have too many files are more than 1000 lines of code, which is a bad code smell.

For example, https://github.com/paritytech/polkadot/blob/master/runtime/common/src/auctions.rs
the tests should be its own file as well as benchmarks.
@GauravDhak
Copy link

i want to work on this issue can you assign me

@ggwpez
Copy link
Member

ggwpez commented Aug 7, 2023 •
edited
Loading

We should wait with large refactors until the monorepo migration finishes on 28th.
Please dont start working on it before that.

@GauravDhak
Copy link

👍

@Sophia-Gold Sophia-Gold transferred this issue from paritytech/polkadot Aug 24, 2023
@the-right-joyce the-right-joyce added I4-refactor Code needs refactoring. and removed I8-refactor labels Aug 25, 2023
@shawntabrizi
Copy link
Member

@GauravDhak should be ready to work on this kind of stuff now.

@the-right-joyce this should be marked easy and good first issue

@bkchr bkchr added the D0-easy Can be fixed primarily by duplicating and adapting code by an intermediate coder. label Mar 11, 2024
@ggwpez ggwpez added C1-mentor A task where a mentor is available. Please indicate in the issue who the mentor could be. C2-good-first-issue A task for a first time contributor to become familiar with the Polkadot-SDK. labels Mar 11, 2024
@GauravDhak
Copy link

I am interested in working on this issue. That's great!

bkchr pushed a commit that referenced this issue Apr 10, 2024
@svyatonik @bkchr
Automated weights for message lane module (#590)
e5f6e30 
* automated weights

* associated WeightInfo type

* update weights using wasmtime

* disable clippy for autogenerated weight.rs

* fix
@Krayt78
Copy link
Contributor

Krayt78 commented Nov 27, 2024

Is this still Available ? Looks like it hasnt been touched since
If it is id like to take it

@shawntabrizi
Copy link
Member

@Krayt78 yes for sure!

Can you please tell us which crates you want to work on, and wait for us to give a thumbs up so there is no wasted effort.

@Krayt78
Copy link
Contributor

Krayt78 commented Nov 27, 2024

@Krayt78 yes for sure!

Can you please tell us which crates you want to work on, and wait for us to give a thumbs up so there is no wasted effort.

Thx :)
I could start with the auction pallet first i guess. I am wondering how it will go merge wise since i already have a PR open to swap its benchmarking to V2. Hopefully the conflict wont be too annoying to fix.

And then once thats done i will ask for more pallets :)

@Krayt78
Copy link
Contributor

Krayt78 commented Dec 2, 2024

can I start on the auction pallet ? @shawntabrizi

@shawntabrizi
Copy link
Member

Go for it!

@Krayt78 Krayt78 mentioned this issue Dec 3, 2024
Merged
@Krayt78
Copy link
Contributor

Krayt78 commented Dec 3, 2024

Hi @shawntabrizi, i have opened a PR regarding the file structure for the auction pallet here : #6746
Let me know if this is good and if you guys would maybe prefer to remove auction.rs to be replaced by a mod.rs in its own folder or if its enough to keep it as is.

@Krayt78
Copy link
Contributor

Krayt78 commented Dec 4, 2024 •
edited
Loading

Seems like both claims.rs and purchase.rs are also 1k+ lines, i can make a PR for each once the previous passes if thats ok

@shawntabrizi
Copy link
Member

@Krayt78 acknowledged. sounds good

This was referenced Dec 6, 2024
Merged
Merged
@Krayt78
Copy link
Contributor

Krayt78 commented Dec 6, 2024

@shawntabrizi i made 2 PR for both claims.rs and purchase.rs here #6780 #6779

I also noticed that other pallets in polkadot-runtime-common have been refactored but not fully (tests are still in mod.rs, resulting in huge file sizes)

example :
paras_registrar
assigned_slots
crowdloan
slots

Can i also refactor those ?

@shawntabrizi
Copy link
Member

@Krayt78 yes, go for it. thank you

@shawntabrizi
Copy link
Member

I have created a small tip for the combination of #6746, #6779, #6780

This was referenced Dec 7, 2024
Merged
Open
Open
Closed
Open
@Krayt78
Copy link
Contributor

Krayt78 commented Dec 7, 2024

I have created a small tip for the combination of #6746, #6779, #6780

thanks a lots Shawn :)

Here are the 4 PRs fo the other 4 pallets : #6783 #6784 #6785 #6787

@Krayt78 Krayt78 mentioned this issue Dec 18, 2024
Closed
github-merge-queue bot pushed a commit that referenced this issue Dec 18, 2024
@Krayt78 @gui1117
Improve pallet claims file structure (#6779)
d8df46c 
Linked to issue #590.
I moved the mod, tests, mock and benchmarking to their own seperate file
to reduce the bloat inside claims.rs

---------

Co-authored-by: Guillaume Thiolliere <[email protected]>
github-merge-queue bot pushed a commit that referenced this issue Dec 19, 2024
@Krayt78 @gui1117 @shawntabrizi
Improve pallet purchase file structure (#6780)
03e5db1 
Linked to issue #590 
I moved the mod, tests, mock and benchmarking to their own seperate file
to reduce the bloat inside purchase.rs

---------

Co-authored-by: Guillaume Thiolliere <[email protected]>
Co-authored-by: Shawn Tabrizi <[email protected]>
github-merge-queue bot pushed a commit that referenced this issue Dec 19, 2024
@Krayt78 @shawntabrizi
Pallet:Auction move tests and benchmark to seperate files (#6746)
0de6854 
# Description

Linked to issue #590.
I moved the tests and benchmarking to their own seperate file to reduce
the bloat inside auctions.rs

Co-authored-by: Shawn Tabrizi <[email protected]>
github-merge-queue bot pushed a commit that referenced this issue Dec 19, 2024
@Krayt78
Improve pallet paras_registrar file structure (#6783)
cbeb66f 
Linked to issue #590 
Extracted code from mod.rs to new tests, mock and benchmarking files
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@GauravDhak GauravDhak

Labels
C1-mentor A task where a mentor is available. Please indicate in the issue who the mentor could be. C2-good-first-issue A task for a first time contributor to become familiar with the Polkadot-SDK. D0-easy Can be fixed primarily by duplicating and adapting code by an intermediate coder. I4-refactor Code needs refactoring.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
7 participants
@xlc @shawntabrizi @bkchr @ggwpez @Krayt78 @the-right-joyce @GauravDhak