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

Support YAML merge syntax #55

Open
michaelbannister opened this issue Dec 2, 2017 · 6 comments
Open

Support YAML merge syntax #55

michaelbannister opened this issue Dec 2, 2017 · 6 comments

Comments

@michaelbannister
Copy link
Contributor

On the back of the handy 'common' section introduced in #32, it would be really handy if this plugin supported the YAML merge key, like this:

common:
  material_yaml_config_plugin: &material_yaml_config_plugin
    git: git@github.com:tomzo/gocd-yaml-config-plugin.git
    auto_update: false
  material_gocd:
    git: git@github.com:gocd/gocd.git
    auto_update: false
    blacklist:
      - externals/**/*.*

pipelines:
  a_pipeline:
  materials:
    yaml_config_plugin:
      <<: *material_yaml_config_plugin
      destination: yaml_config_plugin
    gocd:
      <<: *material_gocd
      destination: gocd

I had a look at yamlbeans recently and it seems it now has support for this, but a release has not been made since then. EsotericSoftware/yamlbeans@095d83d

michaelbannister added a commit to michaelbannister/gocd-yaml-config-plugin that referenced this issue Jan 27, 2018
Fixes tomzo#4

Changed to use recent release of EsotericSoftware's yamlbeans, rather than the
old sanjusoftware fork. This reads yaml mappings into LinkedHashMap, which
preserves ordering.
(It also opens the door for supporting YAML merge key syntax - issue tomzo#55)
michaelbannister added a commit to michaelbannister/gocd-yaml-config-plugin that referenced this issue Jan 27, 2018
Fixes tomzo#4

Changed to use recent release of EsotericSoftware's yamlbeans, rather than the
old sanjusoftware fork. This reads yaml mappings into LinkedHashMap, which
preserves ordering.
(It also opens the door for supporting YAML merge key syntax - issue tomzo#55)
@michaelbannister
Copy link
Contributor Author

I suspect (but have not tested) that this is resolved by #62. Possibly just needs a test or two...

@reegnz
Copy link

reegnz commented Apr 3, 2018

I ran into this issue as well...
And nope, yamlbeans doesn't support the merge syntax:
https://github.com/EsotericSoftware/yamlbeans/blob/master/src/com/esotericsoftware/yamlbeans/tokenizer/Tokenizer.java#L259

Why doesn't this plugin use snakeyaml? It's basically the best yaml parser for java out there, no contest:
https://bitbucket.org/asomov/snakeyaml

@tomzo
Copy link
Owner

tomzo commented Apr 3, 2018

Hi @reegnz

Why doesn't this plugin use snakeyaml? It's basically the best yaml parser for java out there, no contest

That wasn't obvious to me 2 years ago, when choosing a parser. It's not like yamlbeans says in the readme that it does not support this and that.
If you are confident that snakeyaml would be better, then I have no problem with switching to it. If you or anyone can switch the parser and keep the existing tests passing, that would be great.
If you want merge syntax to work, please add at least one test case with use of it.

@reegnz
Copy link

reegnz commented Apr 3, 2018

Hi @tomzo,
I was simply surprized that it didn't use snakeyaml since everywhere you see snakeyaml when you run into yaml parsing in java (eg. https://github.com/FasterXML/jackson-dataformat-yaml). :)
I am pretty confident that snakeyaml is the best lib out there for yaml parsing in java.
We discussed with my colleagues that we might try and contribute this change.

@ryudice
Copy link

ryudice commented Apr 13, 2020

Is this still an issue? I see snakeyaml as a dependency in build.graddle

@reegnz
Copy link

reegnz commented Apr 14, 2020

So is yamlbeans. Checked the code, still using yamlbeans for parsing the config.

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

No branches or pull requests

4 participants