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

5.1 release candidate #2720

Merged
merged 214 commits into from
Oct 4, 2023
Merged

5.1 release candidate #2720

merged 214 commits into from
Oct 4, 2023

Conversation

ZedThree
Copy link
Member

@ZedThree ZedThree commented Jun 14, 2023

We've got into a bit of mess between master and next, and I don't have a great deal of time to sort it out, so the simplest thing is to get everything into master and release 5.1.

New and currently open PRs into next should target this branch now.

Making a New Release of BOUT++

This is checklist of things to do (in order) when making a new
release. This applies equally to both major/minor releases and bugfix
releases

  • Check there are no open issues/PRs for this milestone
    • Fix or bump to next version
  • Make a branch named vX.Y.Z-rc
    • The GitHub repo is setup to protect branches named in this style
    • Major and minor release points (X/Y) should be off
      next. Bugfix releases (Z) should be off master
  • Make pull request into master
    • Any new bugfixes should be PRs into the RC branch, where
      "bugfixes" can include:
      • silencing warnings
      • improving documentation
      • adding tests
  • Run make check-all
    • Raise issues for any tests that fail
  • Possibly run clang-tidy, clang-check, coverity, etc.
  • Review pinned pip package versions for Travis

Before merging PR:

After PR is merged:

dschwoerer and others added 30 commits February 28, 2022 13:34
We need to link with gcov to get gcda files. Further, they are in the
build directories, not the source directories.
Co-authored-by: Peter Hill <peter.hill@york.ac.uk>
* support sudo without password
* set actually a password
* Use --build-args rather then bash script to generate Dockerfile
* next: (517 commits)
  Apply clang-format
  Fix some clang-tidy suggestions in PCR Thomas
  Remove duplicated header in Laplace PCR Thomas
  Fix type of `Coordinates::zlength/dz` in Laplace PCR Thomas
  Fix `LaplacePCR_THOMAS` constructor for new Laplace parameters
  Docs: Fix some formatting issues
  Docs: Fix some broken tables
  Docs: Fix some xBout links
  Docs: Remove outdated note
  Docs: Fix some whitespace
  Docs: Update Ubuntu instructions
  Docs: Explain some common CMake options
  Docs: Add a short quickstart guide with basic instructions
  Docs: Move and expand section on building examples/models with CMake
  Docs: Whitespace cleanup
  Docs: Expand basic CMake docs
  Docs: Note that configure is deprecated
  Docs: Remove out-dated note
  Docs: Use `https` instead of `git` protocol
  Remove some useless forward declarations
  ...
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

include/bout/petsclib.hxx Show resolved Hide resolved
@@ -111,6 +115,14 @@ public:
*/
static void cleanup();

static inline void assertIerr(PetscErrorCode ierr, std::string op = "PETSc operation") {
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: the parameter 'op' is copied for each invocation but only used as a const reference; consider making it a const reference [performance-unnecessary-value-param]

Suggested change
static inline void assertIerr(PetscErrorCode ierr, std::string op = "PETSc operation") {
static inline void assertIerr(PetscErrorCode ierr, const std::string& op = "PETSc operation") {

@@ -111,6 +115,14 @@ public:
*/
static void cleanup();

static inline void assertIerr(PetscErrorCode ierr, std::string op = "PETSc operation") {
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: parameter name 'op' is too short, expected at least 3 characters [readability-identifier-length]

  static inline void assertIerr(PetscErrorCode ierr, std::string op = "PETSc operation") {
                                                                 ^

@@ -111,6 +115,14 @@ public:
*/
static void cleanup();

static inline void assertIerr(PetscErrorCode ierr, std::string op = "PETSc operation") {
if (ierr) {
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: implicit conversion 'PetscErrorCode' (aka 'int') -> bool [readability-implicit-bool-conversion]

Suggested change
if (ierr) {
if (ierr != 0) {

@@ -135,4 +124,17 @@ void PetscLib::setPetscOptions(Options& options, const std::string& prefix) {
}
}
}

BoutException PetscLib::SNESFailure(SNES& snes) {
SNESConvergedReason reason;
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: variable 'reason' is not initialized [cppcoreguidelines-init-variables]

  SNESConvergedReason reason;
                      ^

SNESConvergedReason reason;
BOUT_DO_PETSC(SNESGetConvergedReason(snes, &reason));
#if PETSC_VERSION_GE(3, 15, 0)
const char* message;
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: variable 'message' is not initialized [cppcoreguidelines-init-variables]

Suggested change
const char* message;
const char* message = nullptr;

@bendudson bendudson mentioned this pull request Sep 14, 2023
@ZedThree
Copy link
Member Author

I think this is essentially all the maintenance tasks done and we're ready to go @bendudson

The Fedora tests are still failing, but that is an issue with either Fedora or Github.

@ZedThree
Copy link
Member Author

The python package CI are all failing, I guess because it's looking for a tag 5.1.0 which doesn't exist yet.

@dschwoerer
Copy link
Contributor

Yes, the python package only needs bumping after the release.

If the release is a tag, it should "do the right thing" :-)

bendudson
bendudson previously approved these changes Oct 2, 2023
Copy link
Contributor

@bendudson bendudson left a comment

Choose a reason for hiding this comment

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

Thanks @ZedThree

@ZedThree ZedThree merged commit 2e3cc2c into master Oct 4, 2023
1 check was pending
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.

None yet

4 participants