Skip to content

Conversation

@jvz
Copy link
Member

@jvz jvz commented Dec 7, 2018

This fixes a test to check an empty map using a proper method rather than relying on its toString representation.

No JIRA nor changelog entry, trivial improvement.

Proposed changelog entries

Submitter checklist

- [ ] JIRA issue is well described
- [ ] Changelog entry appropriate for the audience affected by the change (users or developer, depending on the change). Examples
* Use the Internal: prefix if the change has no user-visible impact (API, test frameworks, etc.)
- [ ] Appropriate autotests or explanation to why this change has no tests
- [ ] For dependency updates: links to external changelogs and, if possible, full diffs

Desired reviewers

@reviewbybees

jvz added 2 commits December 7, 2018 14:05
This fixes a test to check an empty map using a proper method rather than relying on its toString representation.
Copy link
Contributor

@Wadeck Wadeck left a comment

Choose a reason for hiding this comment

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

Code is good
Please adjust the description of the PR by removing the non applicable sections instead of letting the default text (removing everything between See [...] full diffs)

@batmat
Copy link
Member

batmat commented Dec 10, 2018

FTR, two tests are failing, and unrelated. Known issue that is being fixed in #3795.

👍 @jvz also as Wadeck said, please complete all fields required in the PR template. I agree here no changelog entry at all is perfect.
Also I recommend to either check a checkbox, or cross the whole item (given generally items are not optional, and showing you saw it but that's it's N/A is better for accelerating reviews)

@batmat batmat added the ready-for-merge The PR is ready to go, and it will be merged soon if there is no negative feedback label Dec 10, 2018
@batmat
Copy link
Member

batmat commented Dec 10, 2018

Did it myself this time to shorten the cycle. But remember it next time :-).

@batmat
Copy link
Member

batmat commented Dec 10, 2018

This looks ready.
I think I will merge this later today, if nobody objects or does this before I do.

Thanks Matt!

@batmat batmat merged commit d60a544 into jenkinsci:master Dec 10, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-merge The PR is ready to go, and it will be merged soon if there is no negative feedback

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants