Skip to content

feat: add tera filter for ordered commit groups#339

Closed
robjtede wants to merge 1 commit into
orhun:mainfrom
robjtede:commit-groups-filter
Closed

feat: add tera filter for ordered commit groups#339
robjtede wants to merge 1 commit into
orhun:mainfrom
robjtede:commit-groups-filter

Conversation

@robjtede
Copy link
Copy Markdown

@robjtede robjtede commented Nov 5, 2023

Description

Add a Tera filter that produces commit groups, with order determined by commit_parsers config.

At least, that is the end goal, the PR does not yet achieve this. It's definitely a mess but I'd like some feedback on the overall goal before progressing further.

Motivation and Context

Using group_by(attribute="group") may not produce desired behavior if order of changelog section matters, eg. "Features" before "Bug Fixes".

As far as I can tell, the most sensible place to source the ordering details is the commit_parsers config (it's what I initially tried to force sections to be ordered differently). It's not wired up like that yet, but hopefully this half of the task shows that it can be done.

How Has This Been Tested?

A new integration test has been added showing that ordering of sections is definable and not accidentally determined by the hash in a regular map.

Screenshots / Logs (if applicable)

Types of Changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation (no code change)
  • Refactor (refactoring production code)
  • Other

Checklist:

  • My code follows the code style of this project.
  • I have updated the documentation accordingly.
  • I have formatted the code with rustfmt.
  • I checked the lints with clippy.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@robjtede robjtede requested a review from orhun as a code owner November 5, 2023 14:21
@welcome
Copy link
Copy Markdown

welcome Bot commented Nov 5, 2023

Thanks for opening this pull request! Please check out our contributing guidelines! ⛰️

@robjtede robjtede marked this pull request as draft November 5, 2023 14:25
@robjtede robjtede changed the title feat: add tera filter for order commit groups feat: add tera filter for ordered commit groups Nov 5, 2023
@orhun
Copy link
Copy Markdown
Owner

orhun commented Nov 14, 2023

This looks good, the question is how can we naturally support this in a non-breaking way. As far as I understood, the user needs to use commit_groups(groups=commit_groups_filter) rather than group_by(attribute="group"). Is that enough? Can you provide more information?

@robjtede
Copy link
Copy Markdown
Author

Yea idea of the non-breaking path is to require a different filter. The current groupBy behavior would remain the same.

@orhun
Copy link
Copy Markdown
Owner

orhun commented Nov 14, 2023

Cool, how far are we from implementing that? I'm not sure if I quite understood the contents of this PR - are you planning to make that happen?

@robjtede
Copy link
Copy Markdown
Author

robjtede commented Nov 14, 2023

It's def not done in the PR yet. You can see the test case for a lil peek at what the PR currently does.

The goal is:

- {% for group, commits in commits | group_by(attribute="group") %}
+ {% for group, commits in commits | commit_groups %}

I'm not 100% sure if it's possible to do it without extra args to the new filter yet.

@orhun
Copy link
Copy Markdown
Owner

orhun commented Dec 4, 2023

I looked into Tera internals and I don't think it is possible to pass an argument to a filter. But as a workaround we can use a static global variable.

I took a stab at this at commit-groups-filter-improved branch. Especially this commit makes it possible to call commit_groups without args.

Let me know if you think this is applicable and we can work on polishing the implementation.

@orhun
Copy link
Copy Markdown
Owner

orhun commented Dec 6, 2023

ping 🐻

@orhun
Copy link
Copy Markdown
Owner

orhun commented Mar 24, 2024

closing for now, I might come back to this in the future

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