Skip to content

CLOUDSTACK-10042: Properly show ICMP SecGroup Rules - #2233

Merged
yadvr merged 1 commit into
apache:4.9from
wido:4.9-secgroup-icmp
Aug 9, 2017
Merged

CLOUDSTACK-10042: Properly show ICMP SecGroup Rules#2233
yadvr merged 1 commit into
apache:4.9from
wido:4.9-secgroup-icmp

Conversation

@wido

@wido wido commented Aug 9, 2017

Copy link
Copy Markdown
Contributor

A simple if-statement would fail if either the type or code were 0
as that if-statement failed them.

By checking if they are defined and casting them to a String afterwards
this makes the if-statement properly resolve and show the rule as it should.

Signed-off-by: Wido den Hollander wido@widodh.nl

A simple if-statement would fail if either the type or code were 0
as that if-statement failed them.

By checking if they are defined and casting them to a String afterwards
this makes the if-statement properly resolve and show the rule as it should.

Signed-off-by: Wido den Hollander <wido@widodh.nl>
@wido
wido requested a review from yadvr August 9, 2017 14:22
@wido wido added this to the 4.9.3 milestone Aug 9, 2017
@yadvr

yadvr commented Aug 9, 2017

Copy link
Copy Markdown
Member

@borisstoyanov can you help validate this, thanks.
@blueorangutan package

@blueorangutan

Copy link
Copy Markdown

@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

@wido

wido commented Aug 9, 2017

Copy link
Copy Markdown
Contributor Author

Thanks @rhtyd ! A screenshot to show it works for me :)

screenshot from 2017-08-09 16-51-28

@blueorangutan

Copy link
Copy Markdown

Packaging result: ✖centos6 ✔centos7 ✔debian. JID-941

@yadvr yadvr left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, did not check the UI myself though.

@yadvr

yadvr commented Aug 9, 2017

Copy link
Copy Markdown
Member

Checked manually, LGTM. Merging.

@yadvr
yadvr merged commit 86b6050 into apache:4.9 Aug 9, 2017
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