DaanHoogland commented on PR #8362: URL: https://github.com/apache/cloudstack/pull/8362#issuecomment-1941443571
> > @JoaoJandre Description in the PR specifically mentions limits for tagged resources. I'll review and make further changes to make it clearer. Regarding listDiskOfferings change I would prefer it in the same PR as it has already been tested internally. We will add relevant test reports. Also, diskoffering suitability for a VM is quite related to offering and storage pool tags. I'll update the title and description for the PR if needed. > > @shwstppr, does the change on the list disk offering depends on the other change you are proposing? Because if one does not affect or depends on the other, they are different contexts, even if related to the same set of functionalities (storage's and host's tags, in this case). > > Mixing contexts in a PR is not encouraged. It makes it difficult for others to review the changes and understand what is being done, especially in these cases with thousands of changes. Furthermore, unrelated changes being tested internally should not be used as reason to open a PR with all of them. While contributing to a community, we should structure our changes and proposals in a way that others can easily understand what is being done/proposed; thus, communicating without noise. @GutoVeronezi (cc @JoaoJandre), I agree with the principal of submitting small isolated chunks of functionality as much as possible. In this case, it is work done for a customer now given back to the upstream source tree. It is indeed slightly different pieces of code and could be separated out, but this is not work we (as in @shwstppr) will do now. It has been tested and any comments on either of the pieces that will lead to further changes, will then have to be tested on the integrated version as well. the QA work is what is leading in this case. I don't like reviewing 7000 lines of code in one go either, sorry. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
