From 13364aa248f22dacaa70ac1e218c6a5da1dc314d Mon Sep 17 00:00:00 2001 From: Manfred Touron Date: Wed, 17 Jul 2019 18:26:28 +0200 Subject: [PATCH 1/2] feat: initial version of pert simplifier --- cmd/pertify/go.sum | 1 + cmd/pertify/main.go | 4 ++ graph.go | 12 +++++- graph_test.go | 2 +- pert_config.go | 101 ++++++++++++++++++++++++++++++++++++++------ pert_config_test.go | 2 +- vertex_test.go | 22 +++++----- viz/graphviz.go | 7 +-- 8 files changed, 120 insertions(+), 31 deletions(-) diff --git a/cmd/pertify/go.sum b/cmd/pertify/go.sum index 8b538fe..131a306 100644 --- a/cmd/pertify/go.sum +++ b/cmd/pertify/go.sum @@ -12,5 +12,6 @@ gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+ gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/urfave/cli.v2 v2.0.0-20180128182452-d3ae77c26ac8 h1:Ggy3mWN4l3PUFPfSG0YB3n5fVYggzysUmiUQ89SnX6Y= gopkg.in/urfave/cli.v2 v2.0.0-20180128182452-d3ae77c26ac8/go.mod h1:cKXr3E0k4aosgycml1b5z33BVV6hai1Kh7uDgFOkbcs= +gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= gopkg.in/yaml.v3 v3.0.0-20190705120443-117fdf03f45f h1:nqBZgl3FUZD7Xe4yitNx/0mqkydzbl4Y89WSTjG6/ok= gopkg.in/yaml.v3 v3.0.0-20190705120443-117fdf03f45f/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= diff --git a/cmd/pertify/main.go b/cmd/pertify/main.go index 1db2c57..f910df2 100644 --- a/cmd/pertify/main.go +++ b/cmd/pertify/main.go @@ -21,6 +21,7 @@ func main() { &cli.BoolFlag{Name: "dot", Usage: "print 'dot' compatible output"}, &cli.BoolFlag{Name: "vertical", Usage: "displaying steps from top to bottom"}, &cli.BoolFlag{Name: "with-details", Usage: "Show pert numbers"}, + &cli.BoolFlag{Name: "no-simplify", Usage: "Don't simplify the graph"}, &cli.BoolFlag{Name: "debug", Aliases: []string{"D"}, Usage: "verbose mode"}, }, Action: graph, @@ -41,6 +42,9 @@ func graph(c *cli.Context) error { return errors.Wrap(err, "failed to parse yaml file") } + if c.Bool("no-simplify") { + config.Opts.NoSimplify = true + } graph := graphman.FromPertConfig(config) if c.Bool("debug") { log.Println(graph) diff --git a/graph.go b/graph.go index 7cb5f72..e49f0dc 100644 --- a/graph.go +++ b/graph.go @@ -214,6 +214,16 @@ func (g *Graph) RemoveVertex(id string) bool { return false } +func (g *Graph) RemoveEdge(src, dst string) bool { + for k, v := range g.edges { + if v.src.id == src && v.dst.id == dst { + g.edges = append(g.edges[:k], g.edges[k+1:]...) + return true + } + } + return false +} + func (g Graph) GetVertex(id string) *Vertex { for _, v := range g.vertices { if v.id == id { @@ -235,7 +245,7 @@ func (g *Graph) AddEdge(srcID, dstID string, attrs ...Attrs) *Edge { dst := g.AddVertex(dstID) edge := newEdge(src, dst, a) src.successors = append(src.successors, edge) - dst.predecessors = append(src.predecessors, edge) + dst.predecessors = append(dst.predecessors, edge) g.edges = append(g.edges, edge) return edge } diff --git a/graph_test.go b/graph_test.go index 4038210..f24d6b7 100644 --- a/graph_test.go +++ b/graph_test.go @@ -23,7 +23,7 @@ func ExampleGraphConnectedSubgraphs() { fmt.Println(subgraph, subgraph.vertices) } // Output: - // {(A,B),(B,C),(C,D),(D,A)} {A,B,C,D} + // {(D,A),(A,B),(B,C),(C,D)} {A,B,C,D} // {(E,E)} {E} // {(F,G),(G,H),(G,I),(H,J),(I,J),(J,K)} {F,G,H,J,K,I} } diff --git a/pert_config.go b/pert_config.go index 6303060..7bc802d 100644 --- a/pert_config.go +++ b/pert_config.go @@ -12,8 +12,18 @@ type PertAction struct { DependsOn []string `yaml:"depends_on"` } +type PertState struct { + ID string `yaml:"id"` + Title string `yaml:"title"` + DependsOn []string `yaml:"depends_on"` +} + type PertConfig struct { Actions []PertAction `yaml:"actions"` + States []PertState `yaml:"states"` + Opts struct { + NoSimplify bool `yaml:"simplify"` + } `yaml:"opts"` } const ( @@ -27,6 +37,21 @@ func FromPertConfig(config PertConfig) *Graph { graph.AddVertex(pertStartVertex) graph.AddVertex(pertFinishVertex) + for _, state := range config.States { + attrs := Attrs{} + if state.Title != "" { + attrs.SetTitle(state.Title) + } + graph.AddVertex(state.ID, attrs) + for _, dependency := range state.DependsOn { + graph.AddEdge( + pertPostID(dependency), + state.ID, + Attrs{}.SetPertZeroTimeActivity(), + ) + } + } + for _, action := range config.Actions { attrs := Attrs{} if action.Title != "" { @@ -46,27 +71,30 @@ func FromPertConfig(config PertConfig) *Graph { } // relationships - postCurrent := fmt.Sprintf("post_%s", action.ID) switch len(action.DependsOn) { case 0: // no dependency, linking with Start - graph.AddEdge(pertStartVertex, postCurrent, attrs) + graph.AddEdge( + pertStartVertex, + pertPostID(action.ID), + attrs, + ) case 1: // only one dependency dependency := action.DependsOn[0] graph.AddEdge( - fmt.Sprintf("post_%s", dependency), - postCurrent, + pertPostID(dependency), + pertPreID(action.ID), attrs, ) default: graph.AddEdge( - fmt.Sprintf("pre_%s", action.ID), - postCurrent, + pertPreID(action.ID), + pertPostID(action.ID), attrs, ) for _, dependency := range action.DependsOn { graph.AddEdge( - fmt.Sprintf("post_%s", dependency), - fmt.Sprintf("pre_%s", action.ID), + pertPostID(dependency), + pertPreID(action.ID), Attrs{}.SetPertZeroTimeActivity(), ) } @@ -75,22 +103,19 @@ func FromPertConfig(config PertConfig) *Graph { // link ending vertices with finish for _, vertex := range graph.SinkVertices() { - if vertex.ID() == pertFinishVertex { + if vertex.id == pertFinishVertex { continue } graph.AddEdge( - vertex.ID(), + vertex.id, pertFinishVertex, Attrs{}.SetPertZeroTimeActivity(), ) } - // optimize - // FIXME: TODO - // nice names for _, vertex := range graph.Vertices() { - if vertex.ID() == pertStartVertex || vertex.ID() == pertFinishVertex { + if vertex.id == pertStartVertex || vertex.id == pertFinishVertex { continue } if vertex.Attrs.GetTitle() == "" { @@ -98,5 +123,53 @@ func FromPertConfig(config PertConfig) *Graph { } } + for _, vertex := range graph.Vertices() { + if vertex.Attrs.GetTitle() == "" { + vertex.Attrs.SetTitle(vertex.id) + } + } + if !config.Opts.NoSimplify { // simplify the graph + verticesToDelete := map[string]bool{} // creating a map so we can iterate over vertices while deleting some entries + for _, vertex := range graph.Vertices() { + if pertIsUntitledState(vertex) || true { + if vertex.OutDegree() == 1 { // remove dummy states with only one dummy successor + successor := vertex.SuccessorEdges()[0] + if pertIsZeroTimeActivity(successor) { + for _, predecessor := range vertex.PredecessorEdges() { + predecessor.dst = successor.dst + } + graph.RemoveEdge(vertex.id, successor.dst.id) + verticesToDelete[vertex.id] = true + } + } + } + } + for id := range verticesToDelete { + graph.RemoveVertex(id) + } + } + return graph } + +func pertPreID(id string) string { return fmt.Sprintf("pre_%s", id) } +func pertPostID(id string) string { return fmt.Sprintf("post_%s", id) } + +func pertIsZeroTimeActivity(edge *Edge) bool { + pert := edge.GetPert() + return pert != nil && pert.IsZeroTimeActivity +} + +func pertIsUntitledState(vertex *Vertex) bool { + pert := vertex.GetPert() + return pert != nil && pert.IsUntitledState +} + +func isMarkedAsDeleted(attrs Attrs) bool { + val, found := attrs["__deleted"].(bool) + return found && val +} + +func markAsDeleted(attrs Attrs) { + attrs["__deleted"] = true +} diff --git a/pert_config_test.go b/pert_config_test.go index 46017b0..7003b75 100644 --- a/pert_config_test.go +++ b/pert_config_test.go @@ -55,5 +55,5 @@ actions: graph := FromPertConfig(config) fmt.Println(graph) // Output: - // {(Start,post_1)[[pert:To=4,Tm=6,Tp=10,Te=6.33,σe=1,Ve=1,title:Prepare foundation]],(Start,post_2)[[pert:To=2,Tm=4,Tp=7,Te=4.17,σe=0.83,Ve=0.69,title:Make & position door frames]],(Start,post_3)[[pert:To=7,Tm=9,Tp=12,Te=9.17,σe=0.83,Ve=0.69,title:Lay drains & floor base]],(post_5,post_4)[[pert:To=2,Tm=4,Tp=5,Te=3.83,σe=0.5,Ve=0.25,title:Install service & settings]],(pre_5,post_5)[[pert:To=7,Tm=10,Tp=15,Te=10.33,σe=1.33,Ve=1.78,title:Erect walls]],(post_1,pre_5)[[pert:]],(post_2,pre_5)[[pert:]],(pre_6,post_6)[[pert:To=1,Tm=2,Tp=4,Te=2.17,σe=0.5,Ve=0.25,title:Plaster ceilings]],(post_4,pre_6)[[pert:]],(post_7,pre_6)[[pert:]],(post_5,post_7)[[pert:To=4,Tm=6,Tp=8,Te=6,σe=0.67,Ve=0.44,title:Erect roof]],(post_7,post_8)[[pert:To=7,Tm=9,Tp=11,Te=9,σe=0.67,Ve=0.44,title:Install door & windows]],(pre_9,post_9)[[pert:To=1,Tm=2,Tp=3,Te=2,σe=0.33,Ve=0.11,title:Fit gutters & pipes]],(post_3,pre_9)[[pert:]],(post_6,pre_9)[[pert:]],(pre_10,post_10)[[pert:To=1,Tm=2,Tp=3,Te=2,σe=0.33,Ve=0.11,title:Paint outside]],(post_8,pre_10)[[pert:]],(post_9,pre_10)[[pert:]],(post_10,Finish)[[pert:]]} + // {(Start,pre_5)[[pert:To=4,Tm=6,Tp=10,Te=6.33,σe=1,Ve=1,title:Prepare foundation]],(Start,pre_5)[[pert:To=2,Tm=4,Tp=7,Te=4.17,σe=0.83,Ve=0.69,title:Make & position door frames]],(Start,pre_9)[[pert:To=7,Tm=9,Tp=12,Te=9.17,σe=0.83,Ve=0.69,title:Lay drains & floor base]],(post_5,Finish)[[pert:To=2,Tm=4,Tp=5,Te=3.83,σe=0.5,Ve=0.25,title:Install service & settings]],(pre_5,post_5)[[pert:To=7,Tm=10,Tp=15,Te=10.33,σe=1.33,Ve=1.78,title:Erect walls]],(pre_6,pre_9)[[pert:To=1,Tm=2,Tp=4,Te=2.17,σe=0.5,Ve=0.25,title:Plaster ceilings]],(post_7,pre_6)[[pert:]],(post_5,Finish)[[pert:To=4,Tm=6,Tp=8,Te=6,σe=0.67,Ve=0.44,title:Erect roof]],(post_7,Finish)[[pert:To=7,Tm=9,Tp=11,Te=9,σe=0.67,Ve=0.44,title:Install door & windows]],(pre_9,pre_10)[[pert:To=1,Tm=2,Tp=3,Te=2,σe=0.33,Ve=0.11,title:Fit gutters & pipes]],(pre_10,Finish)[[pert:To=1,Tm=2,Tp=3,Te=2,σe=0.33,Ve=0.11,title:Paint outside]]} } diff --git a/vertex_test.go b/vertex_test.go index a15d183..ba9ba74 100644 --- a/vertex_test.go +++ b/vertex_test.go @@ -66,19 +66,19 @@ func ExampleVertex_helpers() { // bob // isolated: false // neighbors: {bob,eve,sam} - // predecessor vertices: {sam} - // predecessor edges: {(sam,bob)} + // predecessor vertices: {bob,sam} + // predecessor edges: {(bob,bob),(sam,bob)} // successor vertices: {bob,eve} // successor edges: {(bob,eve),(bob,bob)} - // edges: {(sam,bob),(bob,eve),(bob,bob)} + // edges: {(bob,bob),(sam,bob),(bob,eve),(bob,bob)} // eve // isolated: false - // neighbors: {bob,eve,joy} - // predecessor vertices: {bob,eve,joy} - // predecessor edges: {(bob,eve),(eve,joy),(joy,eve)} + // neighbors: {bob,joy} + // predecessor vertices: {bob,joy} + // predecessor edges: {(bob,eve),(joy,eve)} // successor vertices: {joy} // successor edges: {(eve,joy)} - // edges: {(bob,eve),(eve,joy),(joy,eve),(eve,joy)} + // edges: {(bob,eve),(joy,eve),(eve,joy)} // han // isolated: true // neighbors: {} @@ -89,12 +89,12 @@ func ExampleVertex_helpers() { // edges: {} // joy // isolated: false - // neighbors: {bob,eve} - // predecessor vertices: {bob,eve} - // predecessor edges: {(bob,eve),(eve,joy)} + // neighbors: {eve} + // predecessor vertices: {eve} + // predecessor edges: {(eve,joy)} // successor vertices: {eve} // successor edges: {(joy,eve)} - // edges: {(bob,eve),(eve,joy),(joy,eve)} + // edges: {(eve,joy),(joy,eve)} // sam // isolated: false // neighbors: {bob} diff --git a/viz/graphviz.go b/viz/graphviz.go index 237da75..cd330f2 100644 --- a/viz/graphviz.go +++ b/viz/graphviz.go @@ -30,7 +30,7 @@ func ToGraphviz(g *graphman.Graph, opts *Opts) (string, error) { for _, vertex := range g.Vertices() { if err := gv.AddNode( "G", - vertex.ID(), + escape(vertex.ID()), attrsFromVertex(vertex, opts), ); err != nil { return "", err @@ -38,8 +38,8 @@ func ToGraphviz(g *graphman.Graph, opts *Opts) (string, error) { } for _, edge := range g.Edges() { if err := gv.AddEdge( - edge.Src().ID(), - edge.Dst().ID(), + escape(edge.Src().ID()), + escape(edge.Dst().ID()), true, attrsFromEdge(edge, opts), ); err != nil { @@ -65,6 +65,7 @@ func attrsFromVertex(vertex *graphman.Vertex, opts *Opts) map[string]string { if pert.IsUntitledState { attrs[string(graphviz.Label)] = " " attrs[string(graphviz.Shape)] = "circle" + // attrs[string(graphviz.Style)] = "dashed" } } attrsGeneric(vertex.Attrs, attrs, opts) From 29c3fa4db31dd55e685e27cdea7a6367cb835f38 Mon Sep 17 00:00:00 2001 From: Manfred Touron Date: Thu, 18 Jul 2019 21:07:14 +0200 Subject: [PATCH 2/2] chore: add 'make lint' rule --- .circleci/config.yml | 3 +++ .golangci.yml | 35 +++++++++++++++++++++++++++++++++++ Makefile | 4 ++++ attr.go | 2 +- graph.go | 10 +++++++--- graph_test.go | 4 ++-- path.go | 4 ++-- pert_config.go | 9 --------- 8 files changed, 54 insertions(+), 17 deletions(-) create mode 100644 .golangci.yml diff --git a/.circleci/config.yml b/.circleci/config.yml index ac99caf..d07542c 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -9,4 +9,7 @@ jobs: steps: - checkout - run: go mod download + - run: make install - run: make test + - run: curl -sfL https://install.goreleaser.com/github.com/golangci/golangci-lint.sh | sh -s v1.15.0 + - run: PATH=$PATH:$(pwd)/bin make lint diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 0000000..de6bd49 --- /dev/null +++ b/.golangci.yml @@ -0,0 +1,35 @@ +run: + deadline: 1m + tests: false + #skip-files: + # - ".*\\.gen\\.go" + +linters-settings: + golint: + min-confidence: 0 + maligned: + suggest-new: true + goconst: + min-len: 5 + min-occurrences: 4 + misspell: + locale: US + +linters: + disable-all: true + enable: + - vet + - goconst + - misspell + - deadcode + - misspell + - structcheck + - errcheck + - unused + - varcheck + - staticcheck + - unconvert + - gofmt + - goimports + - golint + - ineffassign diff --git a/Makefile b/Makefile index f34dfc3..98106ab 100644 --- a/Makefile +++ b/Makefile @@ -6,3 +6,7 @@ test: .PHONY: install install: cd cmd/pertify; go install + +.PHONY: lint +lint: + golangci-lint run --verbose ./... diff --git a/attr.go b/attr.go index c254898..b31bdc3 100644 --- a/attr.go +++ b/attr.go @@ -27,7 +27,7 @@ func (a Attrs) IsEmpty() bool { return len(a) == 0 } func (a Attrs) String() string { if a == nil { - return "[INVALID]" + return invalidPlaceholder } if len(a) == 0 { return "[]" diff --git a/graph.go b/graph.go index e49f0dc..972473a 100644 --- a/graph.go +++ b/graph.go @@ -8,6 +8,8 @@ import ( "strings" ) +const invalidPlaceholder = "[INVALID]" + type Graph struct { vertices Vertices edges Edges @@ -60,7 +62,7 @@ func (g Graph) ConnectedSubgraphs() Graphs { func (g Graph) ConnectedSubgraphFromVertex(start *Vertex) *Graph { subgraph := New() visitedEdges := map[*Edge]bool{} - start.WalkAdjacentVertices(func(current, previous *Vertex, depth int) error { + if err := start.WalkAdjacentVertices(func(current, previous *Vertex, depth int) error { subgraph.vertices = append(subgraph.vertices, current) for _, edge := range current.Edges() { if visitedEdges[edge] { @@ -70,7 +72,9 @@ func (g Graph) ConnectedSubgraphFromVertex(start *Vertex) *Graph { subgraph.edges = append(subgraph.edges, edge) } return nil - }) + }); err != nil { + log.Printf("error while walking vertices: %v", err) + } return subgraph } @@ -263,7 +267,7 @@ func (g Graph) IsolatedVertices() Vertices { func (g *Graph) String() string { if g == nil { - return "[INVALID]" + return invalidPlaceholder } elems := []string{} for _, edge := range g.edges { diff --git a/graph_test.go b/graph_test.go index f24d6b7..416518d 100644 --- a/graph_test.go +++ b/graph_test.go @@ -2,7 +2,7 @@ package graphman import "fmt" -func ExampleGraphConnectedSubgraphs() { +func ExampleGraph_ConnectedSubgraphs() { graph := New() // component 1 graph.AddEdge("A", "B") @@ -114,7 +114,7 @@ func ExampleGraph_components() { // shortest: (Y,X,W,V,U) 4 } -func ExampleGraphFindAllPaths() { +func ExampleGraph_FindAllPaths() { graph := New() graph.AddEdge("G", "H") graph.AddEdge("A", "B") diff --git a/path.go b/path.go index 2a511cd..9707cb0 100644 --- a/path.go +++ b/path.go @@ -13,7 +13,7 @@ type Path Edges func (p Path) String() string { if p == nil { - return "[INVALID]" + return invalidPlaceholder } vertices := p.Vertices() ids := []string{} @@ -22,7 +22,7 @@ func (p Path) String() string { } ret := fmt.Sprintf("(%s)", strings.Join(ids, ",")) if !p.IsValid() { - ret += "[INVALID]" + ret += invalidPlaceholder } return ret } diff --git a/pert_config.go b/pert_config.go index 7bc802d..6d531c0 100644 --- a/pert_config.go +++ b/pert_config.go @@ -164,12 +164,3 @@ func pertIsUntitledState(vertex *Vertex) bool { pert := vertex.GetPert() return pert != nil && pert.IsUntitledState } - -func isMarkedAsDeleted(attrs Attrs) bool { - val, found := attrs["__deleted"].(bool) - return found && val -} - -func markAsDeleted(attrs Attrs) { - attrs["__deleted"] = true -}