Skip to content

Conversation

@pyansys-ci-bot
Copy link
Contributor

Update CHANGELOG for v0.68.2 and remove .md files in doc/changelog.d

@pyansys-ci-bot pyansys-ci-bot requested a review from a team as a code owner June 18, 2024 16:20
@ansys-reviewer-bot
Copy link
Contributor

Thanks for opening a Pull Request. If you want to perform a review write a comment saying:

@ansys-reviewer-bot review

@codecov
Copy link

codecov bot commented Jun 18, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 84.21%. Comparing base (7c1eb1e) to head (eb7492c).
Report is 16 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3183      +/-   ##
==========================================
- Coverage   86.63%   84.21%   -2.43%     
==========================================
  Files          52       53       +1     
  Lines        9550     9625      +75     
==========================================
- Hits         8274     8106     -168     
- Misses       1276     1519     +243     

@RobPasMue
Copy link
Member

@germa89 - sections are autogenerated by the action based on the fragment categories. We should not be modifying the content. Otherwise, it defeats the automation purpose. Any enhancements you may want, please open an issue on the actions.

Right now, the changelog action is based on the labeler approach and there are just a few categories. @klmcadams should implement support for branch naming convention or commit prefix as well so that users can decide which option to use. Ideally, the branch/commit style should be more reliable than labels. But it is not implemented yet.

Also, from what I've seen you created the fragments manually. I understand the purpose (i.e. having some complete release notes) but the location of some of the PRs might not be the adequate consequently

I just realized the issue wasn't created - but it was in @klmcadams roadmap. ansys/actions#510

@germa89
Copy link
Collaborator

germa89 commented Jun 19, 2024

@clatapie can we fix this changelog.rst file manually, so we sort the PRs and put them in the right place. Just for this release.

Then, later we can make sure that the documentation PRs are correctly grouped in their category.

I guess we should delete this file if we are going to use the changelog action: https://github.com/ansys/pymapdl/blob/main/.github/release.yml

@RobPasMue we do have Documentation labels. I can create an issue for that. But other option is to "parse" the above release.yml file, which is the file that github reads when the release notes are autogenerated:

image

which is broken.. because we changed all the labels.. FML

@RobPasMue
Copy link
Member

RobPasMue commented Jun 19, 2024

I guess we should delete this file if we are going to use the changelog action: https://github.com/ansys/pymapdl/blob/main/.github/release.yml

@RobPasMue we do have Documentation labels. I can create an issue for that. But other option is to "parse" the above release.yml file, which is the file that github reads when the release notes are autogenerated:

Interesting, I didn't know this existed. Nonetheless, the goal would be to directly inject the autogenerated changelog rather than to delegate it to GitHub - just so that everything is aligned. There is an issue open for this. ansys/actions#460

GitHub
Pythonic interface to MAPDL. Contribute to ansys/pymapdl development by creating an account on GitHub.

@germa89
Copy link
Collaborator

germa89 commented Jun 19, 2024

@RobPasMue this how we have been doing the github release notes in PyMAPDL: https://github.com/ansys/pymapdl/releases/tag/v0.68.0

So the release.yml file is "sort of" github official file. We should probably support it, at the end it is just a YAML file.

GitHub
Hey PyMAPDL users! We've got some exciting updates coming your way with the latest release of our beloved pymapdl package! 🚀 This version do an important effort in improving PyMAPDL documentation a...

@RobPasMue
Copy link
Member

So the release.yml file is "sort of" github official file. We should probably support it, at the end it is just a YAML file.

You are comparing apples and oranges - that's the GitHub release notes, and that's what GitHub will use to generate its release notes automatically, you can keep it if you want, that's up to you. We are using towncrier which is platform agnostic (GitHub, GitLab etc.) to generate release notes. The sections are defined by us via the pyproject.toml file config for towncrier. To standarize it and automate it - and as well, avoid misalignment... - we should be consuming the release notes generated by towncrier IMO.

Why would you want to use towncrier release notes if you are willing to stick to the GitHub release notes?

@germa89
Copy link
Collaborator

germa89 commented Jun 19, 2024

So the release.yml file is "sort of" github official file. We should probably support it, at the end it is just a YAML file.

You are comparing apples and oranges - that's the GitHub release notes, and that's what GitHub will use to generate its release notes automatically, you can keep it if you want, that's up to you. We are using towncrier which is platform agnostic (GitHub, GitLab etc.) to generate release notes. The sections are defined by us via the pyproject.toml file config for towncrier. To standarize it and automate it - and as well, avoid misalignment... - we should be consuming the release notes generated by towncrier IMO.

Why would you want to use towncrier release notes if you are willing to stick to the GitHub release notes?

I think both are "release notes" hence they can be both related and compared. In fact, you mentioned an issue about automatise github release notes too.
But until that is implemented, I want to have the github release notes autogenerated using the github procedure I was doing (based on release.yml).

Additionally, I think town crier building the github release notes from the release.yml file will extend github current approach (quite lacking IMHO). But of course, this is just a suggestion.

@RobPasMue
Copy link
Member

RobPasMue commented Jun 19, 2024

I think both are "release notes" hence they can be both related.

That's why I said apples and oranges, both are fruits but different fruits 😄 technically speaking, they got nothing to do.

In fact, you mentioned an issue about automatise github release notes too. But until that is implemented in the actionlog, I want to have the github release notes completed using the procedure I was doing.

You can keep both for sure (temporarily) - that's fine. You will use towncrier for the automated release notes in the docs and GitHub approach for the GitHub releases.. until we implement it in the action I guess. I wouldn't keep 2 independent release notes fr the long run....

I think having the possibility of town crier to build the github release notes from the release.yml file will extend github current approach (quite lacking IMHO).

But this is where you are probably not understanding the way towncrier works. Towncrier builds your release notes based on your fragments and your towncrier configuration - not based on GitHub's approach. It does not make sense to use that YAML file to build the release notes.

@jorgepiloto
Copy link
Member

One of the problems that Towncrier solves is the bootstrap of the release notes. Even if you could parse that release.yml file, notes would only be generated after releasing the project. Documentation, then, would have to render after releasing the project. This is an issue, what if a build issue arises from a bug in your docs?

We should support Towncrier as much as possible since, as Roberto said, is agnostic. I am not rejecting the GitHub notes, but just saying that we have full control on Towncrier. In addition, we can use these notes later in the GitHub release.

I am in favor of enhancing the sections of the changelog. Also, I would recommend to add some logic based on labels for including and ignoring fragments.

For example, "docs:fragment:skip" could make the changelog action not to run and avoiding adding the fragment. This could be useful in "smol" PRs for fixing typos and similar. It avoids cluttering the CHANGELOG file.

@germa89
Copy link
Collaborator

germa89 commented Jun 19, 2024

I'm quite confident that I'm not being clear...

The file release.yml matches PR with specific labels to a section on the github release notes.

So far, and here is probably where I'm not fully grasping the concepts, towncrier create in the release notes sections such as
Added, Changed, etc... https://mapdl.docs.pyansys.com/version/stable/changelog.html
But how those PR labels are mapped to those sections?? I believe that is hardcoded in here

That is where the release file can be useful. To allow the github repo mantainers to "reuse" that file and have same sections, with same PRs in both, changelog and github release notes.

Does it make sense what I say? Or am I totally losing the point?

@jorgepiloto

One of the problems that Towncrier solves is the bootstrap of the release notes. Even if you could parse that release.yml file, notes would only be generated after releasing the project. Documentation, then, would have to render after releasing the project. This is an issue, what if a build issue arises from a bug in your docs?

The idea is to use the changelog action when github release notes are supported. Until then, I'm happy to do it semi-automatically.
Github release notes can be "auto-generated" if you press a button. And I personally like it that way because it allows you to "add something more" to the release notes, like highlights, or "Main changes" like we do in PyMAPDL. Anyways,

I am in favor of enhancing the sections of the changelog. Also, I would recommend to add some logic based on labels for including and ignoring fragments.

I'm totally in favour of this. I want to also see how we can modify and improve the template.

Pinging @klmcadams for feedback.

@RobPasMue
Copy link
Member

RobPasMue commented Jun 19, 2024

So far, and here is probably where I'm not fully grasping the concepts, towncrier create in the release notes sections such as
Added, Changed, etc... https://mapdl.docs.pyansys.com/version/stable/changelog.html
But how those PR labels are mapped to those sections?? I believe that is hardcoded in here
That is where the release file can be useful. To allow the github repo mantainers to "reuse" that file and have same sections, with same PRs in both, changelog and github release notes.

Hmm gotcha - things are clearer now. We based it on the default labels provided by GitHub projects and the ones shipped by the ansys-templates because those are the labels used by 90% of our projects. Now, that's true - if you have your own labels, this might not work perfectly fine. That's why the commit naming convention and branch naming convention (conventional commits approach) made so much sense, because then we can remove the label handling.

I think I wasn't understanding previously your point - apologies for that part.

Using different labels to categorize the fragments is an option, that's true. However, I still think that the approach is not that standard in that case. Anyway... I was thinking about other options too:

  • Delegating it via configuration to the action: we could open the door to pass in the labels used as arguments... I would still default things to what we currently have, but that way if somebody has custom labels for certain things, they could pass them... Something like:
  changelog-fragment:
    ...
    steps:
    - uses: ansys/actions/doc-changelog@v6
      with:
        token: ${{ secrets.PYANSYS_CI_BOT_TOKEN }}
        added-labels: [enhancement, feature]
        fixed-labels: [bug]
        dependencies-labels: [build, dependencies]
  • Reading from release.yml file this configuration - however this might imply more changes that I am not fully aware right now (especially in regards to the action logic). I'm still not super sure about using this file because then you would have to align the different section names as well...
  • Delegating it to the conventional commits approach based on PR naming- I still believe this is the most reliable approach IMO. Conventional commits is a standard and we know the prefixes allowed and not allowed. Which will simplify things a lot. But this requires implementation as well

To be true - all of them need implementation 😄

@RobPasMue
Copy link
Member

RobPasMue commented Jun 19, 2024

Or a dedicated YAML file - that could also be an option. But I'd avoid this. If this were to be the case, I prefer to delegate it to the release.yml.

If we did choose to use the release.yml file approach, we could also dynamically add/remove fragment sections. As long as they would also be configured in the pyproject.toml, everything should work

@germa89
Copy link
Collaborator

germa89 commented Jun 20, 2024

@RobPasMue

apologies for that part.

No apologies needed :)

Regarding your comment:

  • Delegating it via configuration to the action: ... I dont like to have the sections hardcoded on the action inputs. Even if we are all supposed to have similar sections.
  • Reading from release.yml file this configuration I like this, because it is replicating github approach. And it gives flexibility to the repo mantainers to have their own sections (MAPDL).
  • Delegating it to the conventional commits approach based on PR naming. This could be also fine. The type name written in the PR name should group the PR in the desired category. But I guess you still need a release-like file that match the PR type (p.e. feat:) to their section names.

I think the release-like file is needed if you want to allow custom sections in your change log. Having said that, I see not much difference between using labels or PR names to group the PR under a certain category.

@RobPasMue
Copy link
Member

  • Delegating it to the conventional commits approach based on PR naming. This could be also fine. The type name written in the PR name should group the PR in the desired category. But I guess you still need a release-like file that match the PR type (p.e. feat:) to their section names.

Ah... yes and no. Using the conventional commits would allow us to hardcode the sections so to say, and force all to use the same convention. We really need to push for uniformity @germa89 - we can't have everybody doing whatever they want 😄

@germa89
Copy link
Collaborator

germa89 commented Jun 20, 2024

  • Delegating it to the conventional commits approach based on PR naming. This could be also fine. The type name written in the PR name should group the PR in the desired category. But I guess you still need a release-like file that match the PR type (p.e. feat:) to their section names.

Ah... yes and no. Using the conventional commits would allow us to hardcode the sections so to say, and force all to use the same convention. We really need to push for uniformity @germa89 - we can't have everybody doing whatever they want 😄

Well, I agreee... partially... I do not think we need to reach the level of enforcing certain sections on release notes (should we @MaxJPRey?). At the end of the day, most of us are going to use the same notes, maybe just some sprinkles here and there to fit the project better.

Uniformity is good I agree... but uniformity is not identicalness. All projects are not the same, although the framework is.

@germa89 germa89 enabled auto-merge (squash) June 24, 2024 10:05
@germa89 germa89 merged commit 4cd2fe1 into main Jun 24, 2024
@germa89 germa89 deleted the maint/v0.68.2-changelog branch June 24, 2024 10:22
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.

6 participants