Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

common: ChangeLog supplement for #6117 and #6127 #6136

Merged
merged 2 commits into from
Jan 30, 2025

Conversation

grom72
Copy link
Contributor

@grom72 grom72 commented Jan 21, 2025

Signed-off-by: Tomasz Gromadzki <tomasz.gromadzki@hpe.com>
@grom72 grom72 requested review from janekmi and osalyk January 21, 2025 11:37
@grom72 grom72 changed the title common: changelog suplement for #6117 and #6127 common: ChangeLog supplement for #6117 and #6127 Jan 21, 2025
@grom72 grom72 added the sprint goal This pull request is part of the ongoing sprint label Jan 23, 2025
@grom72 grom72 added this to the 2.1.1 milestone Jan 23, 2025
Copy link
Contributor

@janekmi janekmi left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @grom72 and @osalyk)


ChangeLog line 1 at r1 (raw file):

DoW Mmm DD YYYY Somebody <somebody@somewhere.somewhere>  

ChangeLog line 1 at r1 (raw file):

DoW Mmm DD YYYY Somebody <somebody@somewhere.somewhere>  

I am not so sure having so elaborate placeholder is better. I guess before the release, the best matching line of this kind is just copied over and adjusted. Having to replace a few words instead of just one may be more error prone and IMHO simplifies nothing. Thoughts? @osalyk?


ChangeLog line 7 at r1 (raw file):

	- remove non-Linux support from all compilation paths (OS_KERNEL_NAME)
	- add an aarch64 cross-compilation (requested by DAOS)
	- mute annoying error messages when transactions are intentional abort (#6117)

Or intentional aborts? What does it mean "things are abort"? If you can argue for this case please do because I am sincerely curious.

Suggestion:

mute annoying error messages when transactions are intentionally aborted

ChangeLog line 8 at r1 (raw file):

	- add an aarch64 cross-compilation (requested by DAOS)
	- mute annoying error messages when transactions are intentional abort (#6117)
  	- mute error message "Cannot find any matching device, no bad blocks found" when PMDK is used without PMem (#6127)

There are some spaces before tab. Please remove.


ChangeLog line 8 at r1 (raw file):

	- add an aarch64 cross-compilation (requested by DAOS)
	- mute annoying error messages when transactions are intentional abort (#6117)
  	- mute error message "Cannot find any matching device, no bad blocks found" when PMDK is used without PMem (#6127)

It is good to know this one was not so annoying as the previous one. 😆

Code quote:

error message

Signed-off-by: Tomasz Gromadzki <tomasz.gromadzki@hpe.com>
Copy link
Contributor Author

@grom72 grom72 left a comment

Choose a reason for hiding this comment

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

Reviewable status: 0 of 1 files reviewed, 3 unresolved discussions (waiting on @janekmi and @osalyk)


ChangeLog line 1 at r1 (raw file):

Previously, janekmi (Jan Michalski) wrote…

I am not so sure having so elaborate placeholder is better. I guess before the release, the best matching line of this kind is just copied over and adjusted. Having to replace a few words instead of just one may be more error prone and IMHO simplifies nothing. Thoughts? @osalyk?

Done


ChangeLog line 7 at r1 (raw file):

Previously, janekmi (Jan Michalski) wrote…

Or intentional aborts? What does it mean "things are abort"? If you can argue for this case please do because I am sincerely curious.

Done.


ChangeLog line 8 at r1 (raw file):

Previously, janekmi (Jan Michalski) wrote…

There are some spaces before tab. Please remove.

Done.


ChangeLog line 8 at r1 (raw file):

Previously, janekmi (Jan Michalski) wrote…

It is good to know this one was not so annoying as the previous one. 😆

Done.


ChangeLog line 1 at r1 (raw file):

DoW Mmm DD YYYY Somebody <somebody@somewhere.somewhere>  

Done.

Copy link
Contributor Author

@grom72 grom72 left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 1 files at r1.
Reviewable status: 0 of 1 files reviewed, 3 unresolved discussions (waiting on @janekmi and @osalyk)

Copy link
Contributor

@janekmi janekmi left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 1 of 1 files at r2, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @osalyk)

Copy link
Contributor Author

@grom72 grom72 left a comment

Choose a reason for hiding this comment

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

Reviewed all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @osalyk)

Copy link
Contributor

@osalyk osalyk left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 1 of 1 files at r2, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @grom72)

Copy link
Contributor

@janekmi janekmi left a comment

Choose a reason for hiding this comment

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

Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @grom72)

@janekmi janekmi merged commit b60b28e into pmem:master Jan 30, 2025
5 of 7 checks passed
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
sprint goal This pull request is part of the ongoing sprint
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants