[GitHub] [cloudstack] sureshanaparti commented on pull request #4922: Inclusivity changes for CloudStack - change default git branch name from 'master' to 'main', and rename some offensive words/terms as appropriate

[GitBox <gi...@apache.org>]

sureshanaparti commented on pull request #4922:
URL: https://github.com/apache/cloudstack/pull/4922#issuecomment-831731002


   > 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.
   
   @nathanmcgarvey Thanks for the review, I'll update the changes accordingly. 


-- 
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:
users@infra.apache.org