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

Prepare for the systemd+dbus PR #3816

Merged
merged 8 commits into from
Feb 5, 2025
Merged

Conversation

clumens
Copy link
Contributor

@clumens clumens commented Feb 3, 2025

This splits out a bunch of the prep stuff from #3805 into its own PR so we can get it in faster. I think this is all fairly uncontroversial stuff.

@@ -917,8 +917,8 @@ action_complete(svc_action_t * action)
}
}
}
} else if (pcmk__str_any_of(cmd->action, PCMK_ACTION_MONITOR, PCMK_ACTION_STATUS, NULL) &&
(cmd->interval_ms > 0)) {
} else if (pcmk__str_any_of(cmd->action, PCMK_ACTION_MONITOR, PCMK_ACTION_STATUS, NULL) &&
Copy link
Contributor

Choose a reason for hiding this comment

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

same comment as a497f6a#r1938248123 re: && but whatever, it's preference and was never added to the style guide

With that said, I care more about stylistic consistency than about any particular stylistic preference. As another example, I prefer the opening brace on a separate line for function definitions (which happens to be in our C style guidelines), while your preference seems to be same-line. Ultimately, I don't care, as long as we're consistent about which one we use.

For me, a consistent style helps a lot with the cognitive load required to read and understand the code

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We have indent support in devel/Makefile.am, but indent is kind of old at this point and I know people tend to have various complaints about it. There's also clang-format. Perhaps it'd be worth spending some time configuring that and then making sure everyone's using it in their editors (or we could do a commit hook, github action, etc.)

@clumens clumens added the review: in progress PRs that are currently being reviewed label Feb 3, 2025
Most importantly, replace a tab with spaces so the else blocks line up.
If you scroll up far enough, you'll see that the #endif closes out the
same #ifdef that is immediately used again.
First, invert the test to see if this is a systemd resource.  If it's
not, there's no way goagain was set so we can just bail out to the end
of the function.

Then having done that, everything that was in the block can be
unindented.

No other code changes.
If goagain is not set, just bail to the end of the function.  Having
done that, everything that was in the previous block can be unindented.
I think this is a little more legible.  No other code changes.
...when we disconnect from the bus.  We aren't allowed to close the
connection since we acquired it with dbus_bus_get which makes it a
shared connection.  So, this is the best cleanup we can do.
@nrwahl2 nrwahl2 merged commit 88e9ec1 into ClusterLabs:main Feb 5, 2025
1 check passed
@nrwahl2 nrwahl2 removed the review: in progress PRs that are currently being reviewed label Feb 7, 2025
bmwiedemann pushed a commit to bmwiedemann/openSUSE that referenced this pull request Feb 25, 2025
…R 1248015

https://build.opensuse.org/request/show/1248015
by user yan_gao + dimstar_suse
- Update to version 3.0.0+20250218.3d0ffefd9e:
- build: Fix default pacemaker-remoted path

- Update to version 3.0.0+20250210.af3642fbf6:
- libcrmcluster: prevent external callers from triggering assertion when connecting to cluster (gh#ClusterLabs/pacemaker#3821)

- Update to version 3.0.0+20250207.d06c888ba7:
- libpacemaker: Reset scheduler object in pcmk_simulate()
- libpe_status: Make cluster_status() idempotent
- tools: Fix overflow in crm_simulate --repeat
- libpacemaker: Handle scandir() error in pcmk__profile_dir()
- libpacemaker: Fix mem leak in pcmk__profile_dir()
- tools: Avoid crash in crm_simulate --profile

- Update to version 3.0.0+20250205.88e9ec1325:
- libcrmservices: Unref the dbus connection... (gh#ClusterLabs/pacemaker#3816)
- libcrmservices: Don't leak msg if systemd_p
@clumens clumens deleted the dbus-systemd-prep branch March 10, 2025 16:19
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