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

[2216] Updated README.md for troubleshooting checksum read request rate errors (4-3-stable) #2217

Open
wants to merge 1 commit into
base: 4-3-stable
Choose a base branch
from

Conversation

JustinKyleJames
Copy link
Contributor

This change must be applied to 4.3.4.0 (not 4.3.3.1) since the new setting is not valid without the corresponding updates to iRODS.

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
Copy link
Contributor

@alanking alanking left a comment

Choose a reason for hiding this comment

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

Looks good to me. Let's get another approval before we merge. Also, I suppose we should actually merge the feature before this, too. Also-also, would we have to wait for iRODS 4.3.4 and 4.3.4.0 of this plugin before merging this, since it wouldn't be true until both were released? Hmm...

Copy link
Contributor

@korydraughn korydraughn left a comment

Choose a reason for hiding this comment

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

Also, I suppose we should actually merge the feature before this, too.

Agreed.

Also-also, would we have to wait for iRODS 4.3.4 and 4.3.4.0 of this plugin before merging this, since it wouldn't be true until both were released? Hmm...

That's correct. Let's get this PR to a merge-able point and leave it open until we know what iRODS 4.3.4 looks like. Once this PR is ready, we cherry-pick and merge it into the main branch.

So ...

  1. Polish this PR for approval, but do not merge.
  2. Cherry-pick and merge into the main branch.

README.md Outdated Show resolved Hide resolved
@korydraughn
Copy link
Contributor

Looks like this can be squashed (no pounds) and cherry-picked to 4-3-stable.

@JustinKyleJames
Copy link
Contributor Author

Looks like this can be squashed (no pounds) and cherry-picked to 4-3-stable.

I think you meant to main. I squashed and cherry-picked to main. Will open a PR for it.

@korydraughn korydraughn changed the title [2216] Updated README.md for troubleshooting checksum read request rate errors [2216] Updated README.md for troubleshooting checksum read request rate errors (4-3-stable) Sep 30, 2024
@korydraughn
Copy link
Contributor

Yes, that's what I meant.

@JustinKyleJames
Copy link
Contributor Author

Yes, that's what I meant.

Is it ready for pounding?

@alanking
Copy link
Contributor

alanking commented Oct 1, 2024

The plan is to cherry-pick this to main and merge it there but hold on this one until we're closer to releasing 4.3.4. See #2217 (review). The idea - I think - is that we don't want to advertise the feature when the server version supplying said feature has not yet been released. Alternatively, we can merge now and hope that users view the README for the specific release tag they are using.

@korydraughn
Copy link
Contributor

What @alanking has explained is correct.

Please cherry-pick this work to the main branch and we'll merge that. We'll keep this PR open until we're closer to a 4.3.4 release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants