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

Autumn cleaning part 1 - move transactron #460

Merged
merged 3 commits into from
Oct 5, 2023
Merged

Conversation

lekcyjna123
Copy link
Contributor

Hi,

The new semester is coming so it is good time to start cleaning our repository. This is the first commit to implement #446. I have moved transactron from coreblocks/transactions to transactron directory to allow in future for the easier separation of transactron to its own project/repository. This move required to update all transactron imports in coreblocks and tests directories.

When this MR will be accepted my next step will by to split transactron/lib.py to smaller parts.

@Kristopher38
Copy link
Member

If you're already moving this out of the coreblocks directory tree, why not go all the way through and split it into a separate submodule?

@lekcyjna123
Copy link
Contributor Author

If you create separate submodule, then you have to crate separate review for transactron and its lib and for coreblocks. Whats more it will be hard to manage dependencies between them. So as long as transactron is developed rapidly I would prefer to leave it in coreblocks.

@tilk tilk added the refactor Doesn't change functionality, but makes stuff nicer label Oct 5, 2023
@tilk
Copy link
Member

tilk commented Oct 5, 2023

There are still dependencies from Transactron to Coreblocks.

@lekcyjna123
Copy link
Contributor Author

lekcyjna123 commented Oct 5, 2023

There are still dependencies from Transactron to Coreblocks.

Yes, this is expected. I wanted to use small steps principle to allow for easy review of code. Here I simply drag and drop transactron code, only with changes which are required to don't have regression. Cleaning will be in next PRs.

@lekcyjna123 lekcyjna123 changed the title Autumn cleaning - part 1 Autumn cleaning part 1 - move transactron Oct 5, 2023
Copy link
Member

@Kristopher38 Kristopher38 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you create separate submodule, then you have to crate separate review for transactron and its lib and for coreblocks.

Well this is kind of expected when you work with submodules but I'm not so sure it hinders the work that much.

Whats more it will be hard to manage dependencies between them.

The point is to have transactron be its own separate lib not dependent on coreblocks though (unless you meant something else).

So as long as transactron is developed rapidly I would prefer to leave it in coreblocks.

Is it though? @tilk what do you think?

@tilk
Copy link
Member

tilk commented Oct 5, 2023

Is it though? @tilk what do you think?

Ultimately, I want it to be a separate lib outside of Coreblocks. I have nothing against a small steps approach: let's first prepare for this separation, and later separate using one simple commit.

@tilk tilk merged commit 893202f into master Oct 5, 2023
@tilk tilk deleted the lekcyjna/autumn-cleaning-1 branch October 5, 2023 18:06
github-actions bot pushed a commit that referenced this pull request Oct 5, 2023
tilk pushed a commit to kuznia-rdzeni/transactron that referenced this pull request Nov 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactor Doesn't change functionality, but makes stuff nicer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants