nathanmcgarvey commented on pull request #4922:
URL: https://github.com/apache/cloudstack/pull/4922#issuecomment-831407134
Overall, and this is probably too late for this suggestion: I'd recommend
breaking this out into multiple commits: this is a huge single commit which
makes reverting, diffing, and really all repo maintenance hard.
Detailed suggestions:
1. Carefully evaluate things things that are really version bump ideas that
could break external API compatibility:
E.g. systemvm/debian/opt/cloud/bin/master.py →
systemvm/debian/opt/cloud/bin/router_mode.py
a. Both the renaming of that file, and the changing of the command
line arguments.
b. tools/docker/systemtpl.sh I hope those URLs and files actually
exist immediately after merge!
2. Remove unrelated changes to this PR... don't just tailgate stuff onto an
already huge commit:
plugins/hypervisors/kvm/src/main/java/org/apache/cloudstack/utils/qemu/QemuImg.java
plugins/hypervisors/ovm/src/main/java/com/cloud/ovm/object/Test.java
test/integration/component/test_acl_listsnapshot.py partially
(Non-relevant typos: ("Snapshotss"))
3. Match internal terminology to other external project documentation:
Examples:
a. Instead of "backup/cloudBackup"... call "replica/cloudReplica" to
line up with MySQL docs post 8.0.23 (Ref:
https://dev.mysql.com/doc/refman/8.0/en/binlog-replication-configuration-overview.html)
Found at least in these files:
client/conf/db.properties.in
framework/db/src/main/java/com/cloud/utils/db/TransactionLegacy.java
b. Instead of "Secondary" interface, which has a different meaning,
how about "member" or maybe not even changing it quite yet since most OS
documentation and Linux proper still actually calls them "slaves" in terms of
Linux channel-bonding, by definition. This is going to get really confusing
since if someone says 'secondary' interface, they usually mean a literal second
NIC or alias, not a subordinate interface in a channel-bond/team/whatever.
Found at least in these files:
plugins/hypervisors/ovm3/src/test/java/com/cloud/hypervisor/ovm3/objects/NetworkTest.java
scripts/vm/hypervisor/xenserver/cloud-setup-bonding.sh
...s/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/CitrixResourceBase.java
4. There are missing corrections if you were trying to be 100% code-complete.
Example:
a.
...va/com/cloud/hypervisor/xenserver/resource/wrapper/xenbase/CitrixSetupCommandWrapper.java
still has "slave" as a passed method parameter.
--
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.
For queries about this service, please contact Infrastructure at:
[email protected]
