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

default rules set doesn't appear to be valid yaml #89

Closed
2 tasks done
guykisel opened this issue Jun 21, 2021 · 5 comments · Fixed by #96
Closed
2 tasks done

default rules set doesn't appear to be valid yaml #89

guykisel opened this issue Jun 21, 2021 · 5 comments · Fixed by #96
Labels
bug Something isn't working

Comments

@guykisel
Copy link

Hi, thank you for building this tool! I'm evaluating it for use on my team's project :)

I've been testing out the configuration options, including writing my own rules yaml file. The default rules set (https://github.com/get-woke/woke/blob/main/pkg/rule/default.yaml) noted in the README (https://github.com/get-woke/woke#rules) seemed like a good starting point, since it has many of the same terms and suggested alternatives as my own team would like to use.

However, when I pasted this default rules set into my local .woke.yaml, woke fails with a yaml parsing error - yaml: unmarshal errors:\n line 2: cannot unmarshal !!seq into config.Config. I was able to work around this by adding a top level rules: entry in the yaml, but this definitely confused me for a good 30 minutes or so :)

  • Yes, I've searched similar issues on GitHub and didn't find any.
  • Yes, I've included all information below (version, config, etc).

Please include the following information:

Woke v0.9.1
$ woke --version
# paste output here
woke version 0.9.1
Config file
$ cat .woke.yml
# paste output here

# Most of these are based on https://twitter.com/TwitterEng/status/1278733303508418560
- name: whitelist
  terms:
    - whitelist
    - white-list
    - whitelisted
    - white-listed
  alternatives:
    - allowlist
    - inclusion list
  severity: warning
  note: "The underlying assumption of the whitelist/blacklist metaphor is that white = good and black = bad. Because colors in and of themselves have no predetermined meaning, any meaning we assign to them is cultural: for example, the color red in many Southeast Asian countries is lucky, and is often associated with events like marriages, whereas the color white carries the same connotations in many European countries. In the case of whitelist/blacklist, the terms originate in the publishing industry – one dominated by the USA and England, two countries which participated in slavery and which grapple with their racist legacies to this day."

- name: blacklist
  terms:
    - blacklist
    - black-list
    - blacklisted
    - black-listed
  alternatives:
    - denylist
    - blocklist
    - exclusion list
  severity: warning
  note: "The underlying assumption of the whitelist/blacklist metaphor is that white = good and black = bad. Because colors in and of themselves have no predetermined meaning, any meaning we assign to them is cultural: for example, the color red in many Southeast Asian countries is lucky, and is often associated with events like marriages, whereas the color white carries the same connotations in many European countries. In the case of whitelist/blacklist, the terms originate in the publishing industry – one dominated by the USA and England, two countries which participated in slavery and which grapple with their racist legacies to this day."

- name: master-slave
  terms:
    - master-slave
    - master/slave
  alternatives:
    - leader/follower
    - primary/replica
    - primary/standby

- name: slave
  terms:
    - slave
  alternatives:
    - follower
    - replica
    - standby

- name: grandfathered
  terms:
    - grandfathered
  alternatives:
    - legacy status

- name: man-hours
  terms:
    - man hours
    - man-hours
  alternatives:
    - person hours
    - engineer hours

- name: sanity
  terms:
    - sanity
  alternatives:
    - confidence
    - quick check
    - coherence check

- name: dummy
  terms:
    - dummy
  alternatives:
    - placeholder
    - sample

- name: guys
  terms:
    - guys
  alternatives:
    - folks
    - people
    - you all
    - y'all
    - yinz

- name: whitebox
  terms:
    - white-box
    - whitebox
    - white box
  alternatives:
    - open-box

- name: blackbox
  terms:
    - black-box
    - blackbox
    - black box
  alternatives:
    - closed-box
Go environment
$ go version && go env
# paste output here
go not installed - running windows binaries from https://github.com/get-woke/woke/releases/tag/v0.9.1
Verbose output of running
$ woke --debug
# paste output here
2021-06-21T14:47:42-07:00 DBG woke version 0.9.1 built from 0dc3fe3 on 2021-06-20T17:59:32Z
2021-06-21T14:47:42-07:00 DBG woke completed durationMS=0.6245
Error: yaml: unmarshal errors:
  line 2: cannot unmarshal !!seq into config.Config
Usage:
  woke [globs ...] [flags]

Flags:
  -c, --config string       Config file (default is .woke.yaml in current directory, or $HOME)
      --debug               Enable debug logging
      --exit-1-on-failure   Exit with exit code 1 on failures
  -h, --help                help for woke
      --no-ignore           Ignored files in .gitignore/.wokeignore and inline ignores are processed
  -o, --output string       Output type [text,simple,github-actions,json] (default "text")
      --stdin               Read from stdin
  -v, --version             version for woke

2021-06-21T14:47:42-07:00 FTL  error="yaml: unmarshal errors:\n  line 2: cannot unmarshal !!seq into config.Config"
@guykisel guykisel added the bug Something isn't working label Jun 21, 2021
@caitlinelfring
Copy link
Member

Hi @guykisel! Thanks for the feedback! I can see how that might be confusing. The readme has an example of the yaml structure for the config file in https://github.com/get-woke/woke#rules as well as https://github.com/get-woke/woke/blob/main/example.yaml. I'm curious where you think I can update the docs to make it clearer to hopefully save others from this confusion in the future. Thoughts?

@guykisel
Copy link
Author

guykisel commented Jun 30, 2021

Hmm, instead of updating the docs, would it be possible to update the default rules set to be valid if copied into a rules set? Either that or make a separate default rules set file that does include the top level rules: line, and then link that instead of linking this current default.yaml?

image

Thank you! I'm happy to create a PR for this if that would help.

@caitlinelfring
Copy link
Member

I would prefer not to update default.yaml to match the config file since the default rules are not supposed to be a "config", they are just the definition of rules. Updating the code to support that would end up conflating the intention of the default rules file.

Creating a separate config file as an example with a copy of the default rules wouldn't be DRY and there would be a risk of them being out-of-sync.

I think adding a note near the link to the default rules in the README should suffice, as well as this issue for anyone who might find it in the future

@caitlinelfring
Copy link
Member

#96 hopefully this will help others avoid confusion in the future!

@guykisel
Copy link
Author

guykisel commented Jul 7, 2021

Perfect, thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants