Skip to content
This repository has been archived by the owner on Sep 30, 2024. It is now read-only.

refactor(go mod): migrate to go modules #1356

Merged
merged 8 commits into from
Jul 27, 2021

Conversation

cndoit18
Copy link
Contributor

@cndoit18 cndoit18 commented May 18, 2021

During the migration process, some code is updated directly in the vendor folder, causing inconsistent with the upstream code repository.
I move this part of the code to the migrate
folder.

fixes #1355

Signed-off-by: cndoit18 cndoit18@outlook.com

During the migration process, some code is updated directly in the vendor folder, causing
inconsistent with the upstream code repository.
I move this part of the code to the migrate
directory.

Signed-off-by: cndoit18 <cndoit18@outlook.com>
@cndoit18
Copy link
Contributor Author

In order to completely abolish the vendor, we also need some work to migrate the code from the migrate folder to the official version.

@cndoit18
Copy link
Contributor Author

cc @shlomi-noach

@shlomi-noach
Copy link
Collaborator

Thank you! I can take a look into those changes. Appreciate the kick off!

@cndoit18
Copy link
Contributor Author

cndoit18 commented May 18, 2021

Thank you! I can take a look into those changes. Appreciate the kick off!

Creating a vendor folder is to compatible downstream, if you need, you can delete it to test

I just used go mod vendor.

@cndoit18 cndoit18 marked this pull request as draft May 19, 2021 12:27
@cndoit18
Copy link
Contributor Author

I will try to fix the repair, simply verify the test of CI.

@shlomi-noach
Copy link
Collaborator

thanks again - commit 3d33edf modifies the raft logic - I'd like to avoid that. If we switch to go modules then that's what we should be doing, without logic changes.
I have indeed updated the raft library inline, because my contribution upstream was not accepted. Alas, this is how it has to stay - upstream has diverged since then in a completely different direction, and I don't wish to try and sync with it again.

So I don't mind creating e.g. a openark/raft repo, and use a replace { ... } directive to point to that repo. Same for golib, if needed. What do you think?

@cndoit18
Copy link
Contributor Author

thanks again - commit 3d33edf modifies the raft logic - I'd like to avoid that. If we switch to go modules then that's what we should be doing, without logic changes.
I have indeed updated the raft library inline, because my contribution upstream was not accepted. Alas, this is how it has to stay - upstream has diverged since then in a completely different direction, and I don't wish to try and sync with it again.

So I don't mind creating e.g. a openark/raft repo, and use a replace { ... } directive to point to that repo. Same for golib, if needed. What do you think?

This is too good.

@cndoit18
Copy link
Contributor Author

cndoit18 commented May 20, 2021

dependency on openark/golib#10

@cndoit18 cndoit18 marked this pull request as ready for review May 20, 2021 10:49
@cndoit18
Copy link
Contributor Author

We have successfully switched to go modules!
Next, we can add a cache. same like Dockerfile
And migrated to golang 1.16
It is very big, I think it should be divided into multiple PR.
WDYT @shlomi-noach

https://github.com/presslabs/mysql-operator/pull/681/files

@cndoit18
Copy link
Contributor Author

Of course, we have to ensure that the upstream changes will not affect us after migrating to the go modules.

@shlomi-noach
Copy link
Collaborator

We have successfully switched to go modules!

How did you deal with the changes in raft library?

Next, we can add a cache. same like Dockerfile

Sorry, which cache is that? Do you mean a go mod proxy?

And migrated to golang 1.16

Sorry again, I'm confused. How do you mean?

@cndoit18
Copy link
Contributor Author

We have successfully switched to go modules!

How did you deal with the changes in raft library?

Next, we can add a cache. same like Dockerfile

Sorry, which cache is that? Do you mean a go mod proxy?

And migrated to golang 1.16

Sorry again, I'm confused. How do you mean?

Sorry, please forgive me for bad English, so that there is no expression clear

I use https://github.com/openark/raft, you already have
Docker uses layers when building, if you first use go mod download, the follow-up to build images can be reused.
You are still using golang1.14 image, which can be upgraded to golang1.16.

@cndoit18
Copy link
Contributor Author

@shlomi-noach
Copy link
Collaborator

I use https://github.com/openark/raft, you already have

Can you believe that I completely forgot that I did?

Docker uses layers when building, if you first use go mod download, the follow-up to build images can be reused.

I understand now. Shall we do it in a separate PR?

You are still using golang1.14 image, which can be upgraded to golang1.16.

I understand. We can it now, right?

@cndoit18
Copy link
Contributor Author

I use https://github.com/openark/raft, you already have

Can you believe that I completely forgot that I did?

Docker uses layers when building, if you first use go mod download, the follow-up to build images can be reused.

I understand now. Shall we do it in a separate PR?

You are still using golang1.14 image, which can be upgraded to golang1.16.

I understand. We can it now, right?

Yes, we do it in a separate PR

@cndoit18
Copy link
Contributor Author

You are still using golang1.14 image, which can be upgraded to golang1.16.

I understand. We can it now, right?

Your code has used too many shell scripts. For me, it is very difficult

@shlomi-noach
Copy link
Collaborator

Your code has used too many shell scripts. For me, it is very difficult

No problem. I can do this in a different PR. I am grateful to you for this work.

@cndoit18
Copy link
Contributor Author

Your code has used too many shell scripts. For me, it is very difficult

No problem. I can do this in a different PR. I am grateful to you for this work.

Thank you

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
@shlomi-noach
Copy link
Collaborator

I pushed some final changes to the branch: I ran go mod tidy && go mod vendor and committed the changes.

@cndoit18
Copy link
Contributor Author

I pushed some final changes to the branch: I ran go mod tidy && go mod vendor and committed the changes.

cool

Signed-off-by: cndoit18 <cndoit18@outlook.com>
@cndoit18
Copy link
Contributor Author

I pushed some final changes to the branch: I ran go mod tidy && go mod vendor and committed the changes.

squash commits into one.

@shlomi-noach
Copy link
Collaborator

I'm gonna review this in a few more days and hopefully merge by end of week.

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
@shlomi-noach shlomi-noach merged commit 1b9ee80 into openark:master Jul 27, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Migrating to Go Modules
2 participants