add SampleYAML function #6

Merged
merged 1 commit into from Aug 28, 2015

Conversation

Projects
None yet
3 participants
Owner

rogpeppe commented Aug 28, 2015

This will enable us to generate environment provider boilerplate
YAML files.

+ descr := strings.TrimSpace(f.Description)
+ if descr != "" {
+ section()
+ if !strings.HasSuffix(descr, ".") {
@mhilton

mhilton Aug 28, 2015

Member

Are you sure you want to do this? Sentences might end with a ! or a ? (although that might be unusual for a description)

@urosj

urosj Aug 28, 2015

Owner

+1 on this. Do as little formatting as possible.

@rogpeppe

rogpeppe Aug 28, 2015

Owner

Yeah, I agree (particularly since something might easily end up with an exclamation mark). Done.

+
+const textWidth = 80
+
+func writeSampleDescription(w io.Writer, f Attr, indent string) {
@mhilton

mhilton Aug 28, 2015

Member

It might be useful to have a doc comment here.

+ section()
+}
+
+func emptyLine(w io.Writer, indent string) {
@mhilton

mhilton Aug 28, 2015

Member

A doc comment here would also be good please.

+ return nil
+}
+
+const textWidth = 80
@urosj

urosj Aug 28, 2015

Owner

I know it's just a bit of paperwork to deal with indenting, however, i wonder if this is really required? What if we rather leave this to a user?

@rogpeppe

rogpeppe Aug 28, 2015

Owner

I think it's worth doing, as it means we don't need to bother too much
about formatting and line lengths in the descriptions - they'll all be
formatted consistently (like Go doc comments)

+ f[i], f[j] = f[j], f[i]
+}
+
+func (f fieldsByGroup) Less(i0, i1 int) bool {
@mhilton

mhilton Aug 28, 2015

Member

I think I'd like to see the same parameters for Less as Swap. I don't mind which ones you choose.

+ if canUseSameLine(x) {
+ w.Write(space)
+ } else {
+ w.Write(nl)
@mhilton

mhilton Aug 28, 2015

Member

previously you've used fmt.Fprintf(w, "\n") to do the same thing. I'm not following why this one should be different.

@rogpeppe

rogpeppe Aug 28, 2015

Owner

i've made them consistent

+ fields: environschema.Fields{
+ "foo": {
+ Type: environschema.Tstring,
+ Description: "foo is a string.",
@urosj

urosj Aug 28, 2015

Owner

Test with long description and sentences ending with ? and ! instead of .

@rogpeppe

rogpeppe Aug 28, 2015

Owner

I've removed the full-stop-adding logic.

+ }
+ data = bytes.TrimSuffix(data, nl)
+ lines := bytes.Split(data, nl)
+ for i, line := range lines {
@mhilton

mhilton Aug 28, 2015

Member

any particular reason not to merge this and the above line together?
for i, line := bytes.Split(data, nl) No particular reason to do so either.

@rogpeppe

rogpeppe Aug 28, 2015

Owner

I think it's quite nice to make it clear we have a slice of lines.

+ |# This attribute is immutable and considered secret.
+ |#
+ |# e: ""
+ `,
@urosj

urosj Aug 28, 2015

Owner

What about nested attributes?
level1:
\ \ level2:
\ \ \ \ level3:

@rogpeppe

rogpeppe Aug 28, 2015

Owner

Not possible with the current environment schema.

Owner

urosj commented Aug 28, 2015

Looks ok. I have some reservations about how output is generated (vs if user defines it), but as it is under our control, it shouldn't be a problem.

Member

mhilton commented Aug 28, 2015

👍 with a couple of minor thoughts.

Owner

urosj commented Aug 28, 2015

👍

rogpeppe added a commit that referenced this pull request Aug 28, 2015

@rogpeppe rogpeppe merged commit bd0772b into juju:v1 Aug 28, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment