ocket8888 commented on code in PR #7395:
URL: https://github.com/apache/trafficcontrol/pull/7395#discussion_r1134543857
##########
lib/go-tc/users.go:
##########
@@ -127,9 +127,9 @@ type UserToken struct {
Token string `json:"t"`
}
-// commonUserFields is unexported, but its contents are still visible when it
is embedded
+// CommonUserFields is unexported, but its contents are still visible when it
is embedded
Review Comment:
> CommonUserFields is unexported
is no longer true
also, though, I'm curious why you exported it at all?
##########
lib/go-tc/users.go:
##########
@@ -253,14 +253,14 @@ type UserV40 struct {
Username string `json:"username" db:"username"`
}
-// UsersResponseV4 is the type of a response from Traffic Ops to requests made
+// UsersResponseV4 is the type of response from Traffic Ops to requests made
// to /users which return more than one user for the latest 4.x api version
variant.
type UsersResponseV4 struct {
Response []UserV4 `json:"response"`
Alerts
}
-// UserResponseV4 is the type of a response from Traffic Ops to requests made
+// UserResponseV4 is the type of response from Traffic Ops to requests made
Review Comment:
These two changes aren't any more or less grammatically correct than the
original, but I think the meaning of the word `type` in this context is clearer
before the change.
##########
traffic_ops/traffic_ops_golang/test/helpers.go:
##########
@@ -26,16 +26,54 @@ import (
"strings"
)
-// Extract the tag annotations from a struct into a string array
+// ColsFromStructByTag extracts the tag annotations from a struct into a
string array
func ColsFromStructByTag(tagName string, thing interface{}) []string {
- cols := []string{}
+ return ColsFromStructByTagExclude(tagName, thing, nil)
+}
+
+// InsertAtStr inserts insertMap (string to insert at -> []insert names) into
cols non-destructively.
+func InsertAtStr(cols *[]string, insertMap *map[string][]string) *[]string {
Review Comment:
Maps and slices are reference types - why make all of these types pointers?
Now instead of checking for nil, I have to also make sure the value doesn't
*point* to nil.
##########
traffic_ops/traffic_ops_golang/user/user_test.go:
##########
@@ -0,0 +1,343 @@
+package user
+
+/*
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import (
+ "github.com/apache/trafficcontrol/lib/go-tc"
+ "github.com/apache/trafficcontrol/lib/go-util"
+ "testing"
+ "time"
Review Comment:
same as above
##########
traffic_ops/traffic_ops_golang/test/helpers.go:
##########
@@ -26,16 +26,54 @@ import (
"strings"
)
-// Extract the tag annotations from a struct into a string array
+// ColsFromStructByTag extracts the tag annotations from a struct into a
string array
Review Comment:
would you mind also adding punctuation to the end of this?
##########
traffic_ops/traffic_ops_golang/test/helpers_test.go:
##########
@@ -19,10 +19,14 @@ package test
* under the License.
*/
-import "testing"
+import (
+ "github.com/apache/trafficcontrol/lib/go-util"
+ "testing"
+)
Review Comment:
Imports should be in groups with standard libraries preceding internal
imports, with any third-party libraries in a trailing group.
##########
lib/go-tc/users.go:
##########
@@ -275,7 +275,7 @@ type CurrentUserUpdateRequest struct {
User *CurrentUserUpdateRequestUser `json:"user"`
}
-// CurrentUserUpdateRequestUser holds all of the actual data in a request to
update the current user.
+// CurrentUserUpdateRequestUser holds all the actual data in a request to
update the current user.
Review Comment:
This change makes the comment less grammatically correct than before.
##########
traffic_ops/traffic_ops_golang/user/current_test.go:
##########
@@ -0,0 +1,249 @@
+package user
+
+/*
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import (
+ "context"
+ "errors"
+ "github.com/apache/trafficcontrol/lib/go-tc"
+ "github.com/apache/trafficcontrol/lib/go-util"
+ "github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/test"
+ "github.com/jmoiron/sqlx"
+ "gopkg.in/DATA-DOG/go-sqlmock.v1"
+ "testing"
+ "time"
Review Comment:
same as above
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
