Skip to content
This repository was archived by the owner on Aug 20, 2021. It is now read-only.

Fix set param id attribute - #45

Merged
anweiss merged 1 commit into
docker-archive:masterfrom
minhaj10p:set-param-attr
Jan 16, 2019
Merged

Fix set param id attribute#45
anweiss merged 1 commit into
docker-archive:masterfrom
minhaj10p:set-param-attr

Conversation

@minhaj10p

Copy link
Copy Markdown
Contributor

This PR addresses the discrepancy for SetParam struct: ID got reverted back to id instead of param-id in #40

@anweiss

anweiss commented Jan 15, 2019

Copy link
Copy Markdown
Contributor

Ah my bad @minhaj10p. Can you actually make this change here -> https://github.com/docker/oscalkit/blob/3fb2c16f84b91d7c537bc85078cfca5e727573b4/metaschema/types.tmpl#L17 ... and re-run make generate

@minhaj10p

Copy link
Copy Markdown
Contributor Author

Ah my bad @minhaj10p. Can you actually make this change here ->

oscalkit/metaschema/types.tmpl

Line 17 in 3fb2c16

ID string xml:"id,attr,omitempty" json:"id,omitempty"
... and re-run make generate

It looks like the struct for Profile. Is this the correct place to change?

@anweiss

anweiss commented Jan 15, 2019

Copy link
Copy Markdown
Contributor

lol I'm 0 for 2. Wrong line. It actually needs to be fixed in the range at https://github.com/docker/oscalkit/blob/master/metaschema/types.tmpl#L19. So something like the following:

{{- range .Flags}}
  {{- $cf := commentFlag .Name $m.DefineFlag}}
  {{- range $cf}}
  // {{ . }}
  {{- end}}
  {{- $dt := parseDatatype .Datatype $packageName}}
  {{- if and (eq "id" .Name) (eq "profile" $packageName)}}
  ID string `xml:"param-id,attr,omitempty" json:"id,omitempty"`
  {{- else}}
  {{toCamel .Name}} {{if eq "" $dt}}string{{else}}{{$dt}}{{end}} `xml:"{{ .Name }},attr,omitempty" json:"{{toLowerCamel .Name}},omitempty"`
  {{- end}}
{{- end}}
...

You can run make generate after making changes to the template to verify that it's generating the correct profile.go types.

@minhaj10p

Copy link
Copy Markdown
Contributor Author

@anweiss
The change you suggested changed all id to param-id. Tried with make generate

@anweiss

anweiss commented Jan 15, 2019

Copy link
Copy Markdown
Contributor

@minhaj10p I updated the code sample in my comment with a conditional ... check it again ... sorry for all the edits ... Go templates can be a bit finicky at times

@minhaj10p

Copy link
Copy Markdown
Contributor Author

@anweiss The changes you did are working now! Much thanks for the help. PR updated.

@anweiss

anweiss commented Jan 16, 2019

Copy link
Copy Markdown
Contributor

Awesome!

@anweiss anweiss left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@anweiss
anweiss merged commit 54905ec into docker-archive:master Jan 16, 2019
asadullah-yousuf-10p pushed a commit to asadullah-yousuf-10p/oscalkit that referenced this pull request Jan 18, 2019
@minhaj10p
minhaj10p deleted the set-param-attr branch January 21, 2019 09:41
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants