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

Allow fine-grained control of how release notes are generated #217

Closed
AdmiringWorm opened this issue Mar 16, 2020 · 22 comments
Closed

Allow fine-grained control of how release notes are generated #217

AdmiringWorm opened this issue Mar 16, 2020 · 22 comments
Assignees
Labels
Milestone

Comments

@AdmiringWorm
Copy link
Member

Detailed Description

Currently when publishing a new release, the complete release notes are more or less hard coded in the format that can be used. Would be nice to have the ability to completely change the template used if a user want to have a different format.

Context

Personally I prefer a different kind of format for each release note item that what is currently used in GRM.

Possible Implementation

Possible to handle with an external template file (maybe using handlebar logic) with a fallback to embedded resource files.

Your Environment

  • Version Used: 0.10.0
  • Edition Used (.NET Core, .NET Framework): Any
  • Operating System and version (Windows 10, Ubuntu 18.04): Any

NOTE: I would like to gather feedback on what people would prefer if this was going to be implemented.

@AdmiringWorm
Copy link
Member Author

AdmiringWorm commented Mar 16, 2020

A small example of such a template could perhaps be:

{{#each noteKind}}
  __{{this.kind}}__
  {{#each this.notes}}
    - [{{this.IssueId}}]({{this.IssueUrl}}) {{this.Title}}
  {{/each}
{{/each}

{{#if UseSha}}
// TODO: add same handling here
{{/if}}

Just a minimal example.

NOTE: Not completely familiar with handlebar, so may have syntax incorrect.

@AdmiringWorm
Copy link
Member Author

AdmiringWorm commented Mar 17, 2020

Been checking out scriban instead of handlebars, I'm thinking that may be the one I would suggest instead.

I am at least doing some minor testing of it.

@akordowski
Copy link
Contributor

@gep13 @AdmiringWorm I think this might be also a good solution for #254. Instead having a bunch of switches a template would be more flexible. The definition in yaml colud look like this:

release-notes-templates:
  - name: default
    template: ...
  - name: nuget
    template: ...

The create and export commands would use as default the default template or a --template nuget parameter to use a specific template.

scriban looks very promising in terms of flexibility.

@AdmiringWorm
Copy link
Member Author

@akordowski I don't think it will be a solution for #254, but they may complement each other instead.

One thing to note though, my thoughts were actually not to have the templates specified directly in the YAML file as that would need to re-implement the whole template even just for a very minor change, but rather have several smaller files embedded in GRM which can be overridden by putting a similarly named file in a directory of the users choosing (the template directory would be specified in the YAML file).

For example, first have 1 main file that basically creates the skeleton of the release notes, loading additional files that can be used for overriding the formatting of the release notes section, each release note line, the footer section, and the SHA generation section.

@akordowski
Copy link
Contributor

not to have the templates specified directly in the YAML file as that would need to re-implement the whole template even just for a very minor change

@AdmiringWorm I wonder where do you see the risk of re-implementation? As I see scriban supports if/else, for/foreach etc. So a custom template could be build up in the yaml config without touch the code. Or am I missing someting?

@gep13
Copy link
Member

gep13 commented Aug 1, 2020

@akordowski I guess I am getting a little worried at the complexity/size of the yaml configuration file. I prefer the idea of allowing the creation of additional external template files, which can be used instead of the built in templates.

@akordowski
Copy link
Contributor

@gep13 That's also a good alternative.

@AdmiringWorm
Copy link
Member Author

@akordowski I am not talking about needing to re-implement any code.

Let us say you just want to change one very minor part of the template, and if it is specified in the YAML file (instead of using multiple files as I suggest), you would need to re-implement/re-define the whole template instead of only changing the necessary part.

To make it a bit clearer, using an example from the latest release of GRM.

The current format of each release note line looks something similar to:

- [__#214__](https://github.com/GitTools/GitReleaseManager/issues/214) Add the ability to create/update a release notes that is not a draft

now let us say I don't like that format, and would rather have it look like this:

- Add the ability to create/update a release notes that is not a draft ([__#214__](https://github.com/GitTools/GitReleaseManager/issues/214))

Instead of only change the rule that generates that line, I would need to re-implement the whole release notes template, including the headers, the note line, the footer, the SHA section, and so on.
If these were defined in separate files instead, I could extract only the file I would want to change instead of re-implementing the whole template.
That is the idea, at least.

@akordowski
Copy link
Contributor

@AdmiringWorm I see. But to be honest this seems a complicated to me, rather to have a template for each format. E.g. if I wanted a current format release notes an a plain text release notes I would have only 2 templates, instead of bunch of files for each section, which I also have to tide up together. Hmm, maybe I do't see the big picture here.

@AdmiringWorm
Copy link
Member Author

But to be honest this seems a complicated to me, rather to have a template for each format.

We can't take into all formats of the world, so giving people the option to change the parts they want would be better IMO.

E.g. if I wanted a current format release notes an a plain text release notes I would have only 2 templates, instead of bunch of files for each section, which I also have to tide up together.

In theory, with how I plan to implement it you could extract only the main entry template and make all the changes you want there, instead of doing all the files. It is more about the option to only change a minor part, not to require it.
The plan is, to have 1 file that gets imported by GRM, then that file will use Scriban to import all of the other files for each section. So if that file gets replaced with the logic that doesn't import anything, but rather just have the whole template then additional files won't be needed.
An implementation of the Scriban template loader would be needed for this though.

a small example of the base entry (or skeleton) file would perhaps be something like:

As part of this release we had [{{ issues.count }}]({{ milestone.html_url }}?closed=1) closed.

{{ include 'release-notes.txt' }}

{{
if (config.use_footer)
	include 'footer.txt'
end
}}
{{
if (config.use_sha_section)
	include 'sha-section.txt'
end
}}

^^ Is of course just an example, and honestly, I have no idea yet if it is even valid.
If the above template would then be replaced by a whole template instead, no further files would be processed as it wouldn't include any additional files.

@akordowski
Copy link
Contributor

akordowski commented Aug 1, 2020

I will try to explain what I mean.

default.txt (Markdown)

As part of this release we had [{{ issues.count }}]({{ milestone.html_url }}?closed=1) closed.

{{ for issue in issues }}
- [__#{{ issues.number }}__]({{ issues.url }}) {{ issues.text }}
{{ end }}

{{
if (config.use_footer)
	Footer
end
}}

nuget.txt (Text)

{{ for issue in issues }}
{{ issues.number }} {{ issues.text }}
{{ end }}

xml.txt (XML)

<issues>
{{ for issue in issues }}
<issue>
<issue-number>{{ issues.number }}</issue-number>
<issue-text>{{ issues.text }}</issue-text>
</issue>
{{ end }}
</issues>

... and so on.

With this approach I would only have 3 templates and colud export the release notes dynamically. I don't think I could achieve the same with the solution you proposed.

gitreleasemanager export --template default.txt --variables ...
gitreleasemanager export --template nuget.txt --variables ...
gitreleasemanager export --template xml.txt --variables ...

or

gitreleasemanager export --template default.txt,nuget.txt,xml.txt --variables ...

@AdmiringWorm
Copy link
Member Author

With this approach I would only have 3 templates and colud export the release notes dynamically. I don't think I could achieve the same with the solution you proposed.

Of course you could, there wouldn't be anything stopping that at all.
I have not taken multiple templates into account in the example, because it is not a feature that is currently available in GRM.
When that is something that is supported, there would be nothing stopping the entry template to be changed to instead iterate over a list of requested templates to use.

Or even have GRM call the text template directly even.
One option doesn't automatically exclude the other.

@AdmiringWorm AdmiringWorm self-assigned this Aug 1, 2020
@akordowski
Copy link
Contributor

akordowski commented Aug 1, 2020

Of course you could, there wouldn't be anything stopping that at all.

I really struggle to imagine how

I have not taken multiple templates into account in the example

They would be for #254

@AdmiringWorm
Copy link
Member Author

By re-reading your previous comment, I see now where the confusion lies.

You are specifically talking about exporting release notes, not generating/creating release notes which this issue is about.
The export command takes the existing release notes already generated on GitHub, and exports these to file.
That command can not use these kinds of templates at all, without either triggering the abuse detection or rate-limiting on GitHub (at least not for full release notes, specific milestone release notes the templates can be used for).

The issue you are linking to is to export plain-text release notes, and this is something that would need to be implemented in a way that normalized the markdown and strips out anything that wouldn't make sense to be viewed as normal text.

@akordowski
Copy link
Contributor

You are specifically talking about exporting release notes, not generating/creating release notes which this issue is about.

No, I'm not. I had this approach for both commands (create and export) in mind. The default.txtwould be used to create and export release notes as well.

The export command takes the existing release notes already generated on GitHub, and exports these to file.
... strips out anything that wouldn't make sense to be viewed as normal text.

Yes, I know. As I pointed out in the comment we should refactor the ReleaseNotesExporter to get only the data, so we can avoid markdown stripping and could apply any template we want.

That command can not use these kinds of templates at all, without either triggering the abuse detection or rate-limiting on GitHub (at least not for full release notes, specific milestone release notes the templates can be used for).

Why should the abuse detection or rate-limiting be triggered? In my understanding the request are limited, not the amout of data? So with 2 or 3 requests I could load all data to export release notes for one or all releases. Or am I wrong here?

@AdmiringWorm
Copy link
Member Author

So with 2 or 3 requests I could load all data to export release notes for one or all releases. Or am I wrong here?

I believe so yes, you would need to create first one request to get all of the releases, then another one to get all of the milestones, and after that, you will need to call the API multiple times to get all of the issues that have been closed across the repository life (you don't get absolutely all issues in 1 API call, only a limited set is returned).

The rate limit is unlikely to be hit, but the abuse detection is a different story.

Now to be fair, I could be wrong with what would be necessary, but from my experience with working with raw API calls I do not believe I am. (The octokit library makes some decisions that make multiple API calls transparent).

@akordowski
Copy link
Contributor

According to the GitHub API docs there is a endpoint that loads all issues. So in my understanding loading every issue ist not necessary. But to be sure it would be good to test this endpoint with a repo that has a bunch off issues.

@AdmiringWorm
Copy link
Member Author

That are the old docs (the new ones are here: https://docs.github.com/en/rest/reference/issues?query=is%3Aissue+is%3Aopen+sort%3Aupdated-desc#list-repository-issues), there was a time that you would get absolutely every issue. But not anymore, they are now limited by items per page (can be set to up to 100 issues, which wouldn't be enough for established repositories in the long run when exporting full release notes, only milestone release notes).

@akordowski
Copy link
Contributor

I agree, any kind of limitaions should be keep in mind. Not only GitHub, but other VCS providers as well. The GitHub API allows to fetch the issues over pages. So with a limitation of 5000 requests it were possible (theoretically) to get up to 500.000 issues. I think this is more than enough for a complete release notes export. And even in case off exceed the limit GRM has a sleep function.

The goal for the release notes creation and export should be to provide a flexible system. The current implementation of exporting the already created markdown release notes is indeed very simple. But in this case a logic is needed to strip the markdown to a raw text format. And you don't have other data you might want to export. The point is, that to have the freedom to export release notes in a desired format the raw data is needed.

For me the create and export command are basiclly the same. Both should use raw data. create would use only one template, export could use multiple (to consider the limit rate).

gep13 added a commit that referenced this issue Sep 4, 2020
…-release-notes

(GH-217) Template based release notes creation
@gep13 gep13 closed this as completed Sep 4, 2020
@gep13 gep13 added the Feature label Sep 4, 2020
@gep13 gep13 added this to the 0.12.0 milestone Sep 4, 2020
@gep13 gep13 reopened this Dec 28, 2020
@gep13 gep13 added the Feature label Dec 28, 2020
gep13 added a commit that referenced this issue Dec 30, 2020
(GH-217) Additional improvements to fine-grained templates
@gep13
Copy link
Member

gep13 commented Dec 30, 2020

@AdmiringWorm can we consider this issue closed now, or did you have additional plans for this one?

gittools-bot pushed a commit that referenced this issue Dec 30, 2020
Merge pull request #294 from AdmiringWorm/template-improvements

(GH-217) Additional improvements to fine-grained templates
@AdmiringWorm
Copy link
Member Author

@gep13 I think it is fine to close it now. Anything remaining would probably be part of #288 when that support gets added.

@gittools-bot
Copy link

🎉 This issue has been resolved in version 0.12.0 🎉

The release is available on:

Your GitReleaseManager bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants