search

docker-archive / oscalkit

NIST OSCAL SDK and CLI
https://docker.github.io/oscalkit/
Other
36 stars 23 forks source link

Fix set param id attribute #45

Closed minhaj10p closed 5 years ago

minhaj10p commented 5 years ago

This PR addresses the discrepancy for SetParam struct: ID got reverted back to id instead of param-id in https://github.com/docker/oscalkit/pull/40

anweiss commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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

anweiss commented 5 years ago

@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 commented 5 years ago

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

anweiss commented 5 years ago

Awesome!