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

Template bump cfg #927

Merged
merged 1 commit into from
Nov 19, 2024
Merged

Template bump cfg #927

merged 1 commit into from
Nov 19, 2024

Conversation

mdellweg
Copy link
Member

No description provided.

@mdellweg mdellweg force-pushed the template_bump_cfg branch 3 times, most recently from c41ba1c to 1a4d18e Compare November 18, 2024 23:55
@mdellweg mdellweg marked this pull request as ready for review November 18, 2024 23:55
@mdellweg mdellweg force-pushed the template_bump_cfg branch 5 times, most recently from b370603 to 2d842c4 Compare November 19, 2024 11:03
Copy link
Member

Choose a reason for hiding this comment

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

Arent we getting rid of this?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, but that's a manual process, and we need to care about old branches too.
Switching the bootstrap to pyproject, i thought was a next step for another pr.

Copy link
Member

Choose a reason for hiding this comment

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

Fair

plugin-template Outdated
@@ -487,7 +491,7 @@ def generate_relative_path_set(root_dir):
return applicable_paths


def write_template_to_file(template, plugin_root_dir, relative_path, config):
def template_to_file(template, plugin_root_dir, relative_path, config):
Copy link
Member

Choose a reason for hiding this comment

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

From the pattern of moving to_nice_yaml and creating merge_toml into utils.py this should be moved too.

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmmm, maybe.

Copy link
Member

Choose a reason for hiding this comment

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

🤷

old_toml[merge_key].update(data[merge_key])
tomlkit.dump(old_toml, path.open("w"))


Copy link
Member

Choose a reason for hiding this comment

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

Can you add a docstring minimum example of the desired behavior for this?
So we can glimps into this in the future and know what to expect.

plugin_root_dir,
destination,
template_vars,
)
Copy link
Member

Choose a reason for hiding this comment

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

This (and templates/github/pyproject.toml.tool.j2) means we are managing only the tools section, right?

Copy link
Member Author

Choose a reason for hiding this comment

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

Not exactly. We overwrite all the first level keys in the "tool" section when they are provided here.
We don't mess with other tools yet.
(hence the call to "update".)

Copy link
Member

@pedro-psb pedro-psb left a comment

Choose a reason for hiding this comment

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

Looks good!

"""
Template a file of the form 'basename.toml.merge_key' and combine its content beneath
'merge_key' with the actual 'basename.toml' file.
"""
Copy link
Member

Choose a reason for hiding this comment

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

👍

"ci_update_hour": ci_update_hour,
"ci_update_hour": sum((ord(c) for c in config["plugin_app_label"])) % 24,
"current_version": utils.current_version(plugin_root_path),
"is_pulpdocs_member": config["plugin_name"] in utils.get_pulpdocs_members(),
Copy link
Member

Choose a reason for hiding this comment

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

👍

@mdellweg mdellweg merged commit de50fc9 into pulp:main Nov 19, 2024
11 checks passed
@mdellweg mdellweg deleted the template_bump_cfg branch November 19, 2024 13:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants