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

Document set/get vars in python api #300

Merged

Conversation

inknos
Copy link
Collaborator

@inknos inknos commented Feb 16, 2023

I believe this should be documented in the api, but not as part of the main workflow.

I propose to keep it commented as optional steps to show how to address getting/setting vars, but not as part of the workflow until we create an example that uses it.

Maybe a specific example in the future?

relates to #241

# vars.set('releasever', '33')
#
# Once a variable is set it can be accessed by the getter method
# vars.get('releasever')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe that currently this approach does not work. vars.get('releasever') returns something like a pair, but a structure is unknown in python, therefore you cannot use it at all (bug) . What about to describe - get_priority and get_value? Hope that get priority works

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am sorry - my previous comment was incorrect - get_priority() is not available method for Vars

Get() returns Vars structure that contains

    struct Variable {
        std::string value;
        Priority priority;
    };

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because get() approach is not working - ok working but not useful - I would recommend to remove example with get() and add todo - provide a way how to get option priority. What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@inknos May I ask you what's yours plans?

@inknos inknos force-pushed the ns-update-config-docs branch from 4d1edc9 to 5e7469e Compare March 8, 2023 10:23
@inknos inknos force-pushed the ns-update-config-docs branch from 5e7469e to 185e5fc Compare March 8, 2023 10:24
@j-mracek
Copy link
Contributor

j-mracek commented Mar 8, 2023

LGTM

@j-mracek j-mracek merged commit b9bafe4 into rpm-software-management:main Mar 8, 2023
@inknos inknos deleted the ns-update-config-docs branch June 27, 2023 11:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants