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

[krel] Refactor existing bash scripts into go tools #918

Closed
12 of 16 tasks
saschagrunert opened this issue Nov 1, 2019 · 17 comments
Closed
12 of 16 tasks

[krel] Refactor existing bash scripts into go tools #918

saschagrunert opened this issue Nov 1, 2019 · 17 comments
Assignees
Labels
area/release-eng Issues or PRs related to the Release Engineering subproject kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. kind/feature Categorizes issue or PR as related to a new feature. priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. sig/release Categorizes an issue or PR as relevant to SIG Release.
Milestone

Comments

@saschagrunert
Copy link
Member

saschagrunert commented Nov 1, 2019

The main target is to port the existing bash scripts into golang based versions on top of the existing krel command line tool. The scripts should be added as subcommands to krel whereas common utilities should be re-used over the whole package.

Things to be done

Related: #834, #959

/cc @justaugustus @hasheddan

@justaugustus
Copy link
Member

justaugustus commented Nov 1, 2019

Thanks for opening this! I'm going to come back and shift some things around later as well as drop notes on our #release-management Slack discussion (ref: #869 (comment))

/assign @saschagrunert @justaugustus @hasheddan
/milestone v1.18
/priority important-longterm
/kind feature cleanup
/sig release
/area release-eng

@k8s-ci-robot k8s-ci-robot added the priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. label Nov 1, 2019
@k8s-ci-robot k8s-ci-robot added this to the v1.18 milestone Nov 1, 2019
@k8s-ci-robot k8s-ci-robot added kind/feature Categorizes issue or PR as related to a new feature. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. sig/release Categorizes an issue or PR as relevant to SIG Release. area/release-eng Issues or PRs related to the Release Engineering subproject labels Nov 1, 2019
@justaugustus
Copy link
Member

/assign @saschagrunert @justaugustus @hasheddan

@justaugustus
Copy link
Member

@saschagrunert @hasheddan -- Please work on reconciling pkg/notes/git.go into the pkg/util/git.go and krel push first. Feel free to break up the work however you see fit.

I'll be working on krel ff.

@hasheddan
Copy link
Contributor

Small nitpick: I am of the opinion that we should try to avoid putting things into a util package as it has potential to grow and become bloated with functionality. Currently, the functions in common.go seems like a fair candidate to be part of a util package (although maybe they could go into a cli package or something), but I think the functionality in pkg/notes/git.go should likely go into a pkg/git package which feels more descriptive. My feelings on this are pretty accurately captured here: https://dave.cheney.net/2019/01/08/avoid-package-names-like-base-util-or-common

@saschagrunert
Copy link
Member Author

Small nitpick: I am of the opinion that we should try to avoid putting things into a util package as it has potential to grow and become bloated with functionality. Currently, the functions in common.go seems like a fair candidate to be part of a util package (although maybe they could go into a cli package or something), but I think the functionality in pkg/notes/git.go should likely go into a pkg/git package which feels more descriptive. My feelings on this are pretty accurately captured here: https://dave.cheney.net/2019/01/08/avoid-package-names-like-base-util-or-common

We have the same mind, please take a look at #920

@justaugustus
Copy link
Member

Perfect! This was going to be part of my notes :)

@saschagrunert
Copy link
Member Author

saschagrunert commented Nov 7, 2019

What would be the next steps to move forward here? I suggest to finish the git refactoring (and branch ff adaptions) before porting other scripts. WDYT? Is anyone already working on the git side? (If not, I could)

@saschagrunert
Copy link
Member Author

@hoegaarden regarding the release notes tool we could add another --discover mode like introduced here: 532f7df

@justaugustus justaugustus changed the title Port bash scripts go krel Refactor existing bash scripts into go tools Nov 16, 2019
@saschagrunert
Copy link
Member Author

I think the git package is fine for now, so we can continue to work on ff.go where I expect that more methods from there could go into the git package.

@justaugustus
Copy link
Member

Sascha and Dan have this well in hand, so I'm unassigning myself.
Please still tag me for reviews on these PRs though.
/unassign

@justaugustus
Copy link
Member

anago deprecation announced on k-dev: https://groups.google.com/d/topic/kubernetes-dev/Mhpx-loSBns/discussion

@saschagrunert
Copy link
Member Author

anago deprecation announced on k-dev: https://groups.google.com/d/topic/kubernetes-dev/Mhpx-loSBns/discussion

Alright we should probably start converting anago itself into a krel subcommand. Something like krel run? Then krel could call the internal subcommand functions directly...

@justaugustus justaugustus changed the title Refactor existing bash scripts into go tools [krel] Refactor existing bash scripts into go tools Jan 19, 2020
@justaugustus
Copy link
Member

Alright we should probably start converting anago itself into a krel subcommand. Something like krel run? Then krel could call the internal subcommand functions directly...

@saschagrunert -- Yep. I'm planning on taking the first pass on the anago rewrite before handing it off to y'all for optimizations.

I've begun work on krel gcbmgr here: #1081

@saschagrunert
Copy link
Member Author

Update, the release-notify part is done with #1323

@justaugustus
Copy link
Member

Great progress team!

@saschagrunert
Copy link
Member Author

@k8s-ci-robot
Copy link
Contributor

@saschagrunert: Closing this issue.

In response to this:

We're pretty far. I created new issues for the rest of the migration:

Let's close this one since it has been open for quite a while and we can split-up the work better
/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/release-eng Issues or PRs related to the Release Engineering subproject kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. kind/feature Categorizes issue or PR as related to a new feature. priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. sig/release Categorizes an issue or PR as relevant to SIG Release.
Projects
None yet
Development

No branches or pull requests

4 participants