-
Notifications
You must be signed in to change notification settings - Fork 10
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
Renamed meta to metadata and other cleanups #90
Conversation
olehermanse
commented
Feb 28, 2024
- Python promise modules: Renamed meta parameter to metadata
- Cleaned up README files relating to the updated promise types
- iptables.py: Autoformatting by black
- Bumped version numbers of to be released modules inside python source code
This already confused Ole Petter and myself, so let's use another word that is not identical to 2 existing concepts in CFEngine. (`meta` the promise type and `meta` the promise attribute). Signed-off-by: Ole Herman Schumacher Elgesem <[email protected]>
|
||
If you'd like to develop new promise types (or other modules) for CFEngine, we recommend finishing our getting started tutorial: | ||
|
||
https://docs.cfengine.com/docs/3.21/getting-started.html |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://docs.cfengine.com/docs/3.21/getting-started.html | |
https://docs.cfengine.com/latest/getting-started.html |
Maybe link to latest, lts or docs/master ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe, I did this on purpose to maybe avoid breaking links in the future.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah i always struggle to decide.
|
||
For more detailed information about how custom promise types work, see our documentation: | ||
|
||
https://docs.cfengine.com/docs/3.21/reference-promise-types-custom.html |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here, maybe link to a not specific version
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems pretty ok, just saw one case of markdown formatting used in org formatted document that needs to be fixed. Suggested changing links to not version specific.
Removed version numbers from inside READMEs, since nobody has been updating them. Version numbers are already in the index and also in the python file for each module. Removed other unnecessary / duplicated information which just makes the module pages on build.cfengine.com look slightly worse. Tried making things a bit more consistent and easy to read / understand. Added a bit more information and links for the python promise type library. Tixed some small typos. Signed-off-by: Ole Herman Schumacher Elgesem <[email protected]>
Signed-off-by: Ole Herman Schumacher Elgesem <[email protected]>
… code Signed-off-by: Ole Herman Schumacher Elgesem <[email protected]>
|
||
If you'd like to develop new promise types (or other modules) for CFEngine, we recommend finishing our getting started tutorial: | ||
|
||
https://docs.cfengine.com/docs/3.21/getting-started.html |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah i always struggle to decide.
@@ -182,7 +182,7 @@ def validate_promise(self, promiser: str, attributes: dict, meta: dict): | |||
if command != "flush" and attributes.get("chain") == "ALL": | |||
raise ValidationError("Chain 'ALL' is only available for command 'flush'") | |||
|
|||
def evaluate_promise(self, promiser: str, attributes: dict, meta: dict): | |||
def evaluate_promise(self, promiser: str, attributes: Dict, metadata: Dict): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the reason for using typing.Dict
instead of dict
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't mean anything special by it, I just copy-pasted the function signature from one of the other ones (git promise type I think).
It could make sense to always use the typing ones, make everything look consistent, and they have some more expressive ones. But I don't really have a strong opinion.