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

Refactor machine output to use machineoutput/types.go #2105

Closed
cdrage opened this issue Sep 9, 2019 · 8 comments
Closed

Refactor machine output to use machineoutput/types.go #2105

cdrage opened this issue Sep 9, 2019 · 8 comments
Labels
area/json-output Issues or PRs related to JSON output (machine readable output) area/refactoring Issues or PRs related to code refactoring lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed.

Comments

@cdrage
Copy link
Member

cdrage commented Sep 9, 2019

[kind/Enhancement]

Which functionality do you think we should update/improve?

The below code:

    storageListResultMachineReadable := storage.GetMachineReadableFormatForList(storageListMachineReadable)
  
    if log.IsJSON() {
      out, err := json.Marshal(storageListResultMachineReadable)
      if err != nil {
        return err 
      }   
      fmt.Println(string(out))

Is used throughout odo (https://github.com/openshift/odo/search?q=json.Marshal&unscoped_q=json.Marshal).

This should instead use what we have within: https://github.com/openshift/odo/blob/master/pkg/machineoutput/types.go

Why is this needed?

Clean up / stability

@cdrage cdrage self-assigned this Sep 9, 2019
@kadel
Copy link
Member

kadel commented Sep 11, 2019

This is not a good design. We already have structs representing most of the stuff that will be returned in machine-readable outputs. like components/types.go and url/types.go.

Odo should internally work with structures defined there, and the same structs should also be outputted as jsons.
The only exception should be if we need to return something that is not representations of odo object, like when the operation fails or deletion is successful.
If the operation is listing, or getting info about odo object (project, app, component..) it should always use structs in types.go inside the packages representing a given object.

@cdrage
Copy link
Member Author

cdrage commented Sep 11, 2019

Bit confused @kadel

What I mean is removing the json.Marshal statements for:

https://github.com/openshift/odo/blob/master/pkg/machineoutput/types.go#L34

Which is simply a wrapper around json.Marshal instead of having to copy/paste json.Marshal code everwhere :)

@kadel
Copy link
Member

kadel commented Sep 11, 2019

OH, right. So you mean replacing every occurrence json.Marshal that is outside of machineoutput.OutputSuccess with machineoutput.OutputSuccess, right? That makes sense.

@cdrage
Copy link
Member Author

cdrage commented Sep 12, 2019

@kadel Yup! Exactly.

@kadel
Copy link
Member

kadel commented Dec 5, 2019

/unassign @cdrage
/area json-output

@openshift-ci-robot openshift-ci-robot added the area/json-output Issues or PRs related to JSON output (machine readable output) label Dec 5, 2019
@openshift-bot
Copy link

Issues go stale after 90d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle stale

@openshift-ci-robot openshift-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Mar 4, 2020
@openshift-bot
Copy link

Stale issues rot after 30d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle rotten
/remove-lifecycle stale

@openshift-ci-robot openshift-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Apr 3, 2020
@cdrage
Copy link
Member Author

cdrage commented Apr 7, 2020

I've refactored this as much as I could 👍 I think we can close this for now.

@cdrage cdrage closed this as completed Apr 7, 2020
@rm3l rm3l added the area/refactoring Issues or PRs related to code refactoring label Jun 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/json-output Issues or PRs related to JSON output (machine readable output) area/refactoring Issues or PRs related to code refactoring lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed.
Projects
Archived in project
Development

No branches or pull requests

5 participants