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

Nicer plan diff output. #12136

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Nicer plan diff output. #12136

wants to merge 1 commit into from

Conversation

apollo13
Copy link
Contributor

This commit takes the marker length into account properly so
objects/fields are always at parent + 2 no matter how long the marker
is.

Given this job:

job "example" {
  datacenters = ["dc1"]

  group "cache" {
    network {
      port "db" {
        to = 6379
      }
    }

    service {
      name = "test2"
      tags = [
        "x",
        "c",
      ]
    }

    task "redis" {
      driver = "docker"

      config {
        image = "redis:3.2"

        ports = ["db"]
      }

      resources {
        cpu    = 500
        memory = 256
      }
    }
  }
}

and removing the tags yields (without this patch):

+/- Job: "example"
+/- Task Group: "cache" (1 in-place update)
  +/- Service {
      AddressMode:       "auto"
      EnableTagOverride: "false"
      Name:              "test2"
      Namespace:         "default"
      OnUpdate:          "require_healthy"
      PortLabel:         ""
      TaskName:          ""
    - Tags {
      - Tags: "c"
      - Tags: "x"
      }
      }
      Task: "redis"

and with the patch:

+/- Job: "example"
+/- Task Group: "cache" (1 in-place update)
  +/- Service {
        AddressMode:       "auto"
        EnableTagOverride: "false"
        Name:              "test2"
        Namespace:         "default"
        OnUpdate:          "require_healthy"
        PortLabel:         ""
        TaskName:          ""
      - Tags {
        - Tags: "c"
        - Tags: "x"
        }
      }
      Task: "redis"

Imo the new behavior seems better (ymmv). The patch also right aligns markers which I also find nicer.

@lgfa29 I ran into this while working on #11255 -- do you have any thoughts on this?

This commit takes the marker length into account properly so
objects/fields are always at parent + 2 no matter how long the marker
is.
@apollo13
Copy link
Contributor Author

Also there might be some other occurances where this one is uglier (?) though I do not hope so, let's see what the tests say.

@apollo13 apollo13 marked this pull request as draft February 25, 2022 20:45
@lgfa29
Copy link
Contributor

lgfa29 commented Mar 1, 2022

Your output looks great, so 👍 from me.

With regards to the code, I must admit that these two functions have always being confusing to me 😅

@apollo13
Copy link
Contributor Author

apollo13 commented Mar 2, 2022 via email

@hashicorp-cla
Copy link

hashicorp-cla commented Mar 12, 2022

CLA assistant check
All committers have signed the CLA.

@tgross tgross added the stage/needs-rebase This PR needs to be rebased on main before it can be backported to pick up new BPA workflows label May 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stage/needs-rebase This PR needs to be rebased on main before it can be backported to pick up new BPA workflows
Projects
Development

Successfully merging this pull request may close these issues.

4 participants