Skip to content

Remove some submodules in favour of opam pins #11220

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
Merged
merged 1 commit into from
Jul 22, 2022
Merged

Remove some submodules in favour of opam pins #11220

merged 1 commit into from
Jul 22, 2022

Conversation

balsoft
Copy link
Contributor

@balsoft balsoft commented Jun 8, 2022

Get rid of three submodules, replacing them with opam pins.

Submodules slow down the checkout, and are often annoying when switching branches. Replace them with opam pins.

@balsoft balsoft requested review from bkase, psteckler, imeckler, mrmr1993 and a team as code owners June 8, 2022 09:48
@balsoft balsoft added the ci-build-me Add this label to trigger a circle+buildkite build for this branch label Jun 8, 2022
@balsoft balsoft force-pushed the get-rid-of-some-submodules branch from a9f7061 to 21b7102 Compare June 8, 2022 09:52
@Firobe Firobe added the Tweag label Jun 8, 2022
@balsoft balsoft self-assigned this Jun 8, 2022
@balsoft balsoft force-pushed the get-rid-of-some-submodules branch from 21b7102 to ee4651a Compare June 9, 2022 12:41
@Firobe Firobe linked an issue Jun 9, 2022 that may be closed by this pull request
Decrease amount of git submodules #11212
Closed
@psteckler
Copy link
Contributor

We had async_kernel and rpc_parallel pinned because we had local changes to those libraries.

If we're using stock versions of those libraries, we don't need to pin them, just as we don't pin most other OPAM packages.

Are we discarding those local changes?

@mrmr1993
Copy link
Contributor

mrmr1993 commented Jun 9, 2022

If we're using stock versions of those libraries, we don't need to pin them, just as we don't pin most other OPAM packages.

Are we discarding those local changes?

They're being pinned from our github repos rather than submodules: https://github.com/MinaProtocol/mina/pull/11220/files#diff-4e59d97a11c5f0a15ddcb8dac12467e33c0a2730c176efe8969358b7fd566a28R376.

It makes them more annoying to update, but should reduce friction for everyone else when there are updates (since it's just an opam switch import with no submodule pins).

@psteckler
Copy link
Contributor

They're being pinned from our github repos rather than submodules: #11220 (files).

Ah, thanks for clarifying.

@robinbb
Copy link

robinbb commented Jun 24, 2022

No changes in 14 days, @balsoft ? Does this mean that you are not working on this PR? Perhaps it should be marked as a draft, if so.

@balsoft balsoft marked this pull request as draft June 27, 2022 07:02
@balsoft balsoft force-pushed the get-rid-of-some-submodules branch 2 times, most recently from 9ae57b1 to 316f219 Compare June 28, 2022 10:58
@yorickvP
Copy link
Collaborator

Companion: #11388

@balsoft balsoft force-pushed the get-rid-of-some-submodules branch from 316f219 to 4cd98d8 Compare July 21, 2022 20:46
@balsoft balsoft marked this pull request as ready for review July 22, 2022 09:17
@balsoft balsoft merged commit 7f8d377 into develop Jul 22, 2022
@balsoft balsoft deleted the get-rid-of-some-submodules branch July 22, 2022 09:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mrmr1993 mrmr1993 mrmr1993 approved these changes

@bkase bkase Awaiting requested review from bkase

@psteckler psteckler Awaiting requested review from psteckler

@imeckler imeckler Awaiting requested review from imeckler

Assignees

@balsoft balsoft

Labels
ci-build-me Add this label to trigger a circle+buildkite build for this branch
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Decrease amount of git submodules
6 participants
@balsoft @psteckler @mrmr1993 @robinbb @yorickvP @Firobe