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

Remove deprecated packages #888

Merged
merged 7 commits into from
Aug 14, 2023
Merged

Conversation

ghost
Copy link

@ghost ghost commented Aug 11, 2023

The following packages/functions were deprecated in the repo and has been addressed:

  • io/ioutil
  • strings.Title
  • rand.Seed
  • github.com/golang/protobuf/proto

Copy link
Contributor

@deitch deitch left a comment

Choose a reason for hiding this comment

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

That is a lot of cleanup and modernization.

Copy link
Collaborator

@giggsoff giggsoff left a comment

Choose a reason for hiding this comment

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

It seems we should update base image we use to build go code after moving to new go version (for example I can see potential problem here https://github.com/lf-edge/eden/blob/master/Dockerfile, probably we can find other places with the same problem), can you please check?

@ghost
Copy link
Author

ghost commented Aug 11, 2023

It seems we should update base image we use to build go code after moving to new go version (for example I can see potential problem here https://github.com/lf-edge/eden/blob/master/Dockerfile, probably we can find other places with the same problem), can you please check?

I ran make build-docker , make build and make install all ran successfully. Is there any other way to verify this issue ?

@ghost ghost requested a review from giggsoff August 11, 2023 14:09
@giggsoff
Copy link
Collaborator

It seems we should update base image we use to build go code after moving to new go version (for example I can see potential problem here https://github.com/lf-edge/eden/blob/master/Dockerfile, probably we can find other places with the same problem), can you please check?

I ran make build-docker , make build and make install all ran successfully. Is there any other way to verify this issue ?

It is make build-docker, sorry, it is not so obvious.

@ghost
Copy link
Author

ghost commented Aug 11, 2023

It seems we should update base image we use to build go code after moving to new go version (for example I can see potential problem here https://github.com/lf-edge/eden/blob/master/Dockerfile, probably we can find other places with the same problem), can you please check?

I ran make build-docker , make build and make install all ran successfully. Is there any other way to verify this issue ?

It is make build-docker, sorry, it is not so obvious.

Thanks @giggsoff .

Yes, make build-docker was successful:
https://ctxt.io/2/AABQOoQeEA

@giggsoff
Copy link
Collaborator

It seems we should update base image we use to build go code after moving to new go version (for example I can see potential problem here https://github.com/lf-edge/eden/blob/master/Dockerfile, probably we can find other places with the same problem), can you please check?

I ran make build-docker , make build and make install all ran successfully. Is there any other way to verify this issue ?

It is make build-docker, sorry, it is not so obvious.

Thanks @giggsoff .

Yes, make build-docker was successful: https://ctxt.io/2/AABQOoQeEA

Good to know, thank you!

One question so far: I still can see github.com/golang/protobuf v1.5.2 in go.mod, can we avoid usage of the deprecated package? If it is not so easy, please do go mod tidy to indicate that we use the package indirectly.

@ghost
Copy link
Author

ghost commented Aug 11, 2023

mod

You are right. Thanks, I addressed it.

@giggsoff
Copy link
Collaborator

It looks quite strange that all tests failed so early in the same place, will check a bit later

@giggsoff
Copy link
Collaborator

Well, seems I found the problem @prnvkv-zededa. Please use cases.NoLower option in all your cases.Title calls to work the same way as old strings.Title one.

@ghost
Copy link
Author

ghost commented Aug 14, 2023

Well, seems I found the problem @prnvkv-zededa. Please use cases.NoLower option in all your cases.Title calls to work the same way as old strings.Title one.

Thanks @giggsoff !
Added the cases.Nolower option.

@giggsoff
Copy link
Collaborator

Can you please sign off your commits? I can see that DCO check failed.

Signed-off-by: pranav <pranav@zededa.com>
Signed-off-by: pranav <pranav@zededa.com>
Signed-off-by: pranav <pranav@zededa.com>
Signed-off-by: pranav <pranav@zededa.com>
Signed-off-by: pranav <pranav@zededa.com>
Signed-off-by: pranav <pranav@zededa.com>
Signed-off-by: pranav <pranav@zededa.com>
@giggsoff giggsoff merged commit 9e06f08 into lf-edge:master Aug 14, 2023
5 of 9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants