all: use Go standard spelling for acronyms #6353

Merged
merged 1 commit into from Sep 30, 2016

Conversation

Projects
None yet
4 participants
Owner

rogpeppe commented Sep 29, 2016

The code base is currently inconsistent about whether to use
"Api" or "API", or "Numa" or "NUMA", etc.

This PR changes the code to use the standard Go convention of
capitalising the whole acronym when the first letter is capital.
We've done this for API, NUMA, HTTPS, HTTP, FTP and URL.

It's almost entirely mechanical (the only manual change was in
cmd/modelcmd/base.go where there was an instance variable
named modelApi but a method named modelAPI.

There are some external packages that could also benefit
from this change (notably github.com/juju/utils/proxy).

👍

@rogpeppe rogpeppe changed the title from all: use Go standard spelling for acronyms API and NUMA to all: use Go standard spelling for acronyms Sep 29, 2016

Owner

rogpeppe commented Sep 29, 2016

For reviewers understandably wanting to be careful that the public API hasn't changed,
here's the diff with all test code and packages not under apiserver pruned out:

diff --git a/apiserver/application/application.go b/apiserver/application/application.go
--- a/apiserver/application/application.go
+++ b/apiserver/application/application.go
-   curl, err := charm.ParseURL(args.CharmUrl)
+   curl, err := charm.ParseURL(args.CharmURL)
-   if !args.ForceCharmUrl {
+   if !args.ForceCharmURL {
-   if args.CharmUrl != "" {
+   if args.CharmURL != "" {
-           args.CharmUrl,
+           args.CharmURL,
-           args.ForceCharmUrl,
+           args.ForceCharmURL,
-       args.CharmUrl,
+       args.CharmURL,

diff --git a/apiserver/metricsender/metricsender.go b/apiserver/metricsender/metricsender.go
--- a/apiserver/metricsender/metricsender.go
+++ b/apiserver/metricsender/metricsender.go
-   defaultSender            MetricSender = &HttpSender{}
+   defaultSender            MetricSender = &HTTPSender{}

diff --git a/apiserver/metricsender/sender.go b/apiserver/metricsender/sender.go
--- a/apiserver/metricsender/sender.go
+++ b/apiserver/metricsender/sender.go
-// HttpSender is the default used for sending
+// HTTPSender is the default used for sending
-type HttpSender struct {
+type HTTPSender struct {
-func (s *HttpSender) Send(metrics []*wireformat.MetricBatch) (*wireformat.Response, error) {
+func (s *HTTPSender) Send(metrics []*wireformat.MetricBatch) (*wireformat.Response, error) {

diff --git a/apiserver/params/params.go b/apiserver/params/params.go
--- a/apiserver/params/params.go
+++ b/apiserver/params/params.go
-   CharmUrl         string                         `json:"charm-url"`
+   CharmURL         string                         `json:"charm-url"`
-   CharmUrl        string             `json:"charm-url"`
-   ForceCharmUrl   bool               `json:"force-charm-url"`
+   CharmURL        string             `json:"charm-url"`
+   ForceCharmURL   bool               `json:"force-charm-url"`
-   // CharmUrl is the new url for the charm.
-   CharmUrl string `json:"charm-url"`
+   // CharmURL is the new url for the charm.
+   CharmURL string `json:"charm-url"`
Owner

rogpeppe commented Sep 30, 2016

I have also verified that nothing in the API has changed with my jujuapidoc tool.
Here's the diff it reported between two JSON descriptions of the entire Juju API.

--- /tmp/api1.json  2016-09-30 10:02:15.285693322 +0100
+++ /tmp/api2.json  2016-09-30 10:02:27.813688132 +0100
@@ -5708,7 +5708,7 @@
                        }
                    }
                    {
-                       Name: "CharmUrl"
+                       Name: "CharmURL"
                        Tag: "json:\"charm-url\""
                        Type: {
                            Kind: "string"
@@ -5996,7 +5996,7 @@
                        }
                    }
                    {
-                       Name: "CharmUrl"
+                       Name: "CharmURL"
                        Tag: "json:\"charm-url\""
                        Type: {
                            Kind: "string"
@@ -6327,7 +6327,7 @@
                        }
                    }
                    {
-                       Name: "CharmUrl"
+                       Name: "CharmURL"
                        Tag: "json:\"charm-url\""
                        Type: {
                            Kind: "string"
@@ -6335,7 +6335,7 @@
                        }
                    }
                    {
-                       Name: "ForceCharmUrl"
+                       Name: "ForceCharmURL"
                        Tag: "json:\"force-charm-url\""
                        Type: {
                            Kind: "bool"

That would have also included parts of the API defined outside the apiserver hierarchy if they had changed.

axw approved these changes Sep 30, 2016

LGTM, thank you.

Owner

rogpeppe commented Sep 30, 2016

$$merge$$

Contributor

jujubot commented Sep 30, 2016

Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju

Contributor

jujubot commented Sep 30, 2016

Build failed: Tests failed
build url: http://juju-ci.vapour.ws:8080/job/github-merge-juju/9382

all: use Go standard spelling for acronyms
The code base is currently inconsistent about whether to use
"Api" or "API", or "Numa" or "NUMA", etc.

This PR changes the code to use the standard Go convention of
capitalising the whole acronym when the first letter is capital.
We've done this for API, NUMA, HTTPS, HTTP, FTP and URL.

It's almost entirely mechanical (the only manual change was in
cmd/modelcmd/base.go where there was an instance variable
named modelApi but a method named modelAPI.

There are some external packages that could also benefit
from this change (notably github.com/juju/utils/proxy).
Owner

rogpeppe commented Sep 30, 2016

$$merge$$

Contributor

jujubot commented Sep 30, 2016

Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju

Contributor

jujubot commented Sep 30, 2016

Build failed: Tests failed
build url: http://juju-ci.vapour.ws:8080/job/github-merge-juju/9384

Owner

rogpeppe commented Sep 30, 2016

$$merge$$

Contributor

jujubot commented Sep 30, 2016

Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju

@jujubot jujubot merged commit d0ff839 into juju:master Sep 30, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment