Skip to content

Conversation

rhusar
Copy link
Member

@rhusar rhusar commented Feb 16, 2024

@rhusar rhusar requested a review from jfclere February 16, 2024 18:34
Copy link
Member

@jajik jajik left a comment

Choose a reason for hiding this comment

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

I've never noticed this. 😄

Just a nitpick: could you please update the second commit message to something like test: Use proper EnableMCMPReceive option?

@rhusar
Copy link
Member Author

rhusar commented Feb 19, 2024

I've never noticed this. 😄

Just a nitpick: could you please update the second commit message to something like test: Use proper EnableMCMPReceive option?

Ah, right, we talked about the commit message prefix and forgot to add it myself... Fixed now.

@rhusar rhusar requested a review from jajik February 19, 2024 14:33
Copy link
Member

@jajik jajik left a comment

Choose a reason for hiding this comment

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

Thanks, LGTM now.

@rhusar

This comment was marked as resolved.

@jajik
Copy link
Member

jajik commented Apr 8, 2024

@rhusar We're going to need a rebase here.

@jajik
Copy link
Member

jajik commented Apr 8, 2024

Also, is this something we would like to backport?

@rhusar
Copy link
Member Author

rhusar commented Apr 8, 2024

@rhusar We're going to need a rebase here.

Done, but I am afraid there will be many more rebases :)

@rhusar
Copy link
Member Author

rhusar commented Apr 8, 2024

Also, is this something we would like to backport?

I think we have to - otherwise this will be causing a lot of confusion when applying 'newer' config on legacy versions. But lets get this merged first upstream, that's a de facto requirement.

@jajik
Copy link
Member

jajik commented Apr 8, 2024

@rhusar We're going to need a rebase here.

Done, but I am afraid there will be many more rebases :)

Once tests pass, I would be for merging it if @jfclere is not against it.

Also, is this something we would like to backport?

I think we have to - otherwise this will be causing a lot of confusion when applying 'newer' config on legacy versions. But lets get this merged first upstream, that's a de facto requirement.

That's the answer I expected :) it will be easier documentation wise as well

@jajik
Copy link
Member

jajik commented Apr 9, 2024

Merging this one.

@jajik jajik merged commit c3fa781 into modcluster:main Apr 9, 2024
@rhusar rhusar deleted the MODCLUSTER-551 branch April 9, 2024 11:17
@rhusar
Copy link
Member Author

rhusar commented Apr 9, 2024

Thanks @jajik and @jfclere

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.

3 participants