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.

Sign up for GitHub

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

Jump to bottom

Code modernization #587

Merged
merged 14 commits into from
Sep 16, 2022
Merged

Code modernization #587

merged 14 commits into from
Sep 16, 2022

Conversation

offa
Copy link
Contributor

@offa offa commented Sep 16, 2022

Some code modernization.

  • Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
  • Ensure that the pull request title represents the desired changelog entry
  • Please describe what you did
  • Link to relevant issues in GitHub or Jira
  • Link to relevant pull requests, esp. upstream and downstream changes
  • Ensure you have provided tests - that demonstrates feature works or fixes the issue

@timja timja added the chore For changelog: A maintenance chore with no functional changes label Sep 16, 2022
timja
timja approved these changes Sep 16, 2022
View reviewed changes
Copy link
Member

@timja timja left a comment

Choose a reason for hiding this comment

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

nice!

offa
offa commented Sep 16, 2022
View reviewed changes
src/test/java/org/jenkinsci/remoting/protocol/ProtocolStackImplTest.java Outdated
} catch (IOException e) {
assertThat(e.getMessage(), containsString("Broken pipe"));
}
final IOException e = assertThrows(IOException.class, () -> pipe.sink().write(ByteBuffer.allocate(1)));
Copy link
Contributor Author

@offa offa Sep 16, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

This change breaks the Windows CI build somehow; Linux works here though.

Copy link
Member

Choose a reason for hiding this comment

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

This change breaks

Therefore reverted in commit c336c91.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks!

@basil basil merged commit 26e2449 into jenkinsci:master Sep 16, 2022
@offa offa deleted the modernize_code branch September 17, 2022 12:39
basil added a commit to basil/jenkins that referenced this pull request Aug 22, 2023
@basil
Side-port test cleanup from jenkinsci/remoting#587
517764d
@basil basil mentioned this pull request Aug 22, 2023
Merged
NotMyFault pushed a commit to jenkinsci/jenkins that referenced this pull request Aug 29, 2023
@basil
Side-port test cleanup from jenkinsci/remoting#587 (#8414)
6e39c5b
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@timja timja timja approved these changes

@basil basil basil approved these changes

Assignees
No one assigned
Labels
chore For changelog: A maintenance chore with no functional changes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@offa @basil @timja