You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by Darren Shepherd <da...@gmail.com> on 2013/09/27 19:43:41 UTC

Review Request 14360: Make getName() match the constant names for Network Capability, and Service

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14360/
-----------------------------------------------------------

Review request for cloudstack, Alena Prokharchyk and Chiradeep Vittal.


Repository: cloudstack-git


Description
-------

VPCVirtualRouter is still left as inconsitent as changing that name will be impactful.  The unit test checked in will still ensure that future providers match though


Diffs
-----

  api/src/com/cloud/network/Network.java 49f380b 
  api/test/com/cloud/network/NetworkTest.java PRE-CREATION 

Diff: https://reviews.apache.org/r/14360/diff/


Testing
-------


Thanks,

Darren Shepherd


Re: Review Request 14360: Make getName() match the constant names for Network Capability, and Service

Posted by Darren Shepherd <da...@gmail.com>.

> On Sept. 27, 2013, 8:21 p.m., Alena Prokharchyk wrote:
> > Darren, changes in Network.java are going to break API compatibility as capabilities parameters are used in createNetworkOffering API call (serviceCapabilitystList param). CS UI and third party UI will get impacted as well.
> > If we have to change the name, its gotta be the capability parameter name, not its name value.
> 
> daan Hoogland wrote:
>     This in turn, could have impact on other uses, like in a db could it?
>     
>     @Darren: you will need a lot more then just a (unit)test for the getname()
> 
> Alena Prokharchyk wrote:
>     Won't impact the DB as there is no direct ref between capability name and DB field name /value.

I already discarded this.  I didn't realize the current string values are what we desire and the Java constant names that are out of sync are really the "typos."  So I have no complaint with the current implementation and changing the constant and all references to it would be a waste of time.


- Darren


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14360/#review26447
-----------------------------------------------------------


On Sept. 27, 2013, 5:43 p.m., Darren Shepherd wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14360/
> -----------------------------------------------------------
> 
> (Updated Sept. 27, 2013, 5:43 p.m.)
> 
> 
> Review request for cloudstack, Alena Prokharchyk and Chiradeep Vittal.
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> -------
> 
> VPCVirtualRouter is still left as inconsitent as changing that name will be impactful.  The unit test checked in will still ensure that future providers match though
> 
> 
> Diffs
> -----
> 
>   api/src/com/cloud/network/Network.java 49f380b 
>   api/test/com/cloud/network/NetworkTest.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/14360/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Darren Shepherd
> 
>


Re: Review Request 14360: Make getName() match the constant names for Network Capability, and Service

Posted by Darren Shepherd <da...@gmail.com>.
I discarded the patch.  The purpose of throwing it up on review board was so that people could comment on the impact.  And the impact is bad.  

Darren

> On Sep 28, 2013, at 4:43 AM, "daan Hoogland" <da...@gmail.com> wrote:
> 
> 
> This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/14360/
> 
> On September 27th, 2013, 8:21 p.m. UTC, Alena Prokharchyk wrote:
> 
> Darren, changes in Network.java are going to break API compatibility as capabilities parameters are used in createNetworkOffering API call (serviceCapabilitystList param). CS UI and third party UI will get impacted as well.
> If we have to change the name, its gotta be the capability parameter name, not its name value.
> This in turn, could have impact on other uses, like in a db could it?
> 
> @Darren: you will need a lot more then just a (unit)test for the getname()
> 
> - daan
> 
> 
> On September 27th, 2013, 5:43 p.m. UTC, Darren Shepherd wrote:
> 
> Review request for cloudstack, Alena Prokharchyk and Chiradeep Vittal.
> By Darren Shepherd.
> Updated Sept. 27, 2013, 5:43 p.m.
> 
> Repository: cloudstack-git
> Description
> 
> VPCVirtualRouter is still left as inconsitent as changing that name will be impactful.  The unit test checked in will still ensure that future providers match though
> Diffs
> 
> api/src/com/cloud/network/Network.java (49f380b)
> api/test/com/cloud/network/NetworkTest.java (PRE-CREATION)
> View Diff

Re: Review Request 14360: Make getName() match the constant names for Network Capability, and Service

Posted by daan Hoogland <da...@gmail.com>.

> On Sept. 27, 2013, 8:21 p.m., Alena Prokharchyk wrote:
> > Darren, changes in Network.java are going to break API compatibility as capabilities parameters are used in createNetworkOffering API call (serviceCapabilitystList param). CS UI and third party UI will get impacted as well.
> > If we have to change the name, its gotta be the capability parameter name, not its name value.

This in turn, could have impact on other uses, like in a db could it?

@Darren: you will need a lot more then just a (unit)test for the getname()


- daan


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14360/#review26447
-----------------------------------------------------------


On Sept. 27, 2013, 5:43 p.m., Darren Shepherd wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14360/
> -----------------------------------------------------------
> 
> (Updated Sept. 27, 2013, 5:43 p.m.)
> 
> 
> Review request for cloudstack, Alena Prokharchyk and Chiradeep Vittal.
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> -------
> 
> VPCVirtualRouter is still left as inconsitent as changing that name will be impactful.  The unit test checked in will still ensure that future providers match though
> 
> 
> Diffs
> -----
> 
>   api/src/com/cloud/network/Network.java 49f380b 
>   api/test/com/cloud/network/NetworkTest.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/14360/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Darren Shepherd
> 
>


Re: Review Request 14360: Make getName() match the constant names for Network Capability, and Service

Posted by Alena Prokharchyk <al...@citrix.com>.

> On Sept. 27, 2013, 8:21 p.m., Alena Prokharchyk wrote:
> > Darren, changes in Network.java are going to break API compatibility as capabilities parameters are used in createNetworkOffering API call (serviceCapabilitystList param). CS UI and third party UI will get impacted as well.
> > If we have to change the name, its gotta be the capability parameter name, not its name value.
> 
> daan Hoogland wrote:
>     This in turn, could have impact on other uses, like in a db could it?
>     
>     @Darren: you will need a lot more then just a (unit)test for the getname()

Won't impact the DB as there is no direct ref between capability name and DB field name /value.


- Alena


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14360/#review26447
-----------------------------------------------------------


On Sept. 27, 2013, 5:43 p.m., Darren Shepherd wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14360/
> -----------------------------------------------------------
> 
> (Updated Sept. 27, 2013, 5:43 p.m.)
> 
> 
> Review request for cloudstack, Alena Prokharchyk and Chiradeep Vittal.
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> -------
> 
> VPCVirtualRouter is still left as inconsitent as changing that name will be impactful.  The unit test checked in will still ensure that future providers match though
> 
> 
> Diffs
> -----
> 
>   api/src/com/cloud/network/Network.java 49f380b 
>   api/test/com/cloud/network/NetworkTest.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/14360/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Darren Shepherd
> 
>


Re: Review Request 14360: Make getName() match the constant names for Network Capability, and Service

Posted by Alena Prokharchyk <al...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14360/#review26447
-----------------------------------------------------------


Darren, changes in Network.java are going to break API compatibility as capabilities parameters are used in createNetworkOffering API call (serviceCapabilitystList param). CS UI and third party UI will get impacted as well.
If we have to change the name, its gotta be the capability parameter name, not its name value.

- Alena Prokharchyk


On Sept. 27, 2013, 5:43 p.m., Darren Shepherd wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14360/
> -----------------------------------------------------------
> 
> (Updated Sept. 27, 2013, 5:43 p.m.)
> 
> 
> Review request for cloudstack, Alena Prokharchyk and Chiradeep Vittal.
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> -------
> 
> VPCVirtualRouter is still left as inconsitent as changing that name will be impactful.  The unit test checked in will still ensure that future providers match though
> 
> 
> Diffs
> -----
> 
>   api/src/com/cloud/network/Network.java 49f380b 
>   api/test/com/cloud/network/NetworkTest.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/14360/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Darren Shepherd
> 
>