ansys / pymapdl

Pythonic interface to MAPDL
https://mapdl.docs.pyansys.com
MIT License
419 stars 116 forks source link

chore: update CHANGELOG for v0.68.2 #3183

Closed pyansys-ci-bot closed 1 week ago

pyansys-ci-bot commented 2 weeks ago

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

ansys-reviewer-bot[bot] commented 2 weeks ago

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

@ansys-reviewer-bot review

codecov[bot] commented 2 weeks ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

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

Additional details and impacted files ```diff @@ 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 commented 1 week ago

@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. https://github.com/ansys/actions/issues/510

germa89 commented 1 week ago

@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 commented 1 week ago

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. https://github.com/ansys/actions/issues/460

GitHub
pymapdl/.github/release.yml at main ยท ansys/pymapdl
Pythonic interface to MAPDL. Contribute to ansys/pymapdl development by creating an account on GitHub.
germa89 commented 1 week ago

@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
Release v0.68.0 ยท ansys/pymapdl
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 commented 1 week ago

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 commented 1 week ago

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 commented 1 week ago

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 commented 1 week ago

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 commented 1 week ago

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.

Release notes โ€” PyMAPDL
RobPasMue commented 1 week ago

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:

  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]

To be true - all of them need implementation ๐Ÿ˜„

RobPasMue commented 1 week ago

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 commented 1 week ago

@RobPasMue

apologies for that part.

No apologies needed :)

Regarding your comment:

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 commented 1 week ago
  • 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 commented 1 week ago
  • 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.