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

YAMBi: YAML Plugin Based on Parser Generated by Bison #2225

Merged
merged 53 commits into from Sep 10, 2018

Conversation

Projects
None yet
2 participants
@sanssecours
Contributor

sanssecours commented Sep 5, 2018

Checklist

  • I added unit tests.
  • I ran all tests locally and everything went fine.
  • I added code comments and logging statements.
  • The metadata of the plugin should be up to date.
@markus2330

Looks very nice! I love the names of your plugins. Only small comments.

int elektraCppTemplateGet (Plugin * handle, KeySet * ks, Key * parentKey);
int elektraCppTemplateSet (Plugin * handle, KeySet * ks, Key * parentKey);
int elektraCppTemplateError (Plugin * handle, KeySet * ks, Key * parentKey);
int elektraCppTemplateGet (Plugin * handle, KeySet * returned, Key * parentKey);

This comment has been minimized.

@markus2330

markus2330 Sep 8, 2018

Contributor

@tom-wa recently asked why we call this parameter "returned".

I wonder why you call the parameter differently for "Error"?

@markus2330

markus2330 Sep 8, 2018

Contributor

@tom-wa recently asked why we call this parameter "returned".

I wonder why you call the parameter differently for "Error"?

This comment has been minimized.

@sanssecours

sanssecours Sep 9, 2018

Contributor

I wonder why you call the parameter differently for "Error"?

I basically just copied the variable names from the template plugin. It does look like the variable names are different in the header and the source files of the template plugin too (ks vs. returned).

@sanssecours

sanssecours Sep 9, 2018

Contributor

I wonder why you call the parameter differently for "Error"?

I basically just copied the variable names from the template plugin. It does look like the variable names are different in the header and the source files of the template plugin too (ks vs. returned).

This comment has been minimized.

@markus2330

markus2330 Sep 9, 2018

Contributor

Thank you for fixing this! Yes, it should be consistent. I am okay with either name.

@markus2330

markus2330 Sep 9, 2018

Contributor

Thank you for fixing this! Yes, it should be consistent. I am okay with either name.

Show outdated Hide outdated src/plugins/yambi/CMakeLists.txt
Show outdated Hide outdated src/plugins/yambi/README.md
Show outdated Hide outdated src/plugins/yambi/README.md
Show outdated Hide outdated src/plugins/yambi/convert.cpp
Show outdated Hide outdated src/plugins/yambi/input.hpp
Show outdated Hide outdated src/plugins/yambi/driver.hpp
Show outdated Hide outdated src/plugins/yambi/lexer.cpp
Show outdated Hide outdated src/plugins/yambi/symbol.hpp
Show outdated Hide outdated src/plugins/yambi/symbol.hpp

sanssecours added some commits Sep 1, 2018

sanssecours added some commits Sep 5, 2018

Travis: Exclude plugins from `🍏 Clang ASAN`
This update should improve the runtime of the build job.

@markus2330 markus2330 merged commit f3afc13 into ElektraInitiative:master Sep 10, 2018

2 checks passed

continuous-integration/jenkins/pr-merge This commit looks good
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@markus2330

This comment has been minimized.

Show comment
Hide comment
@markus2330

markus2330 Sep 10, 2018

Contributor

Thank you!

Contributor

markus2330 commented Sep 10, 2018

Thank you!

@sanssecours sanssecours deleted the sanssecours:🦌 branch Sep 10, 2018

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