Skip to content

Use MixedCase constant names #110

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 12 commits into from
Nov 7, 2016
Merged

Use MixedCase constant names #110

merged 12 commits into from
Nov 7, 2016

Conversation

strk
Copy link
Member

@strk strk commented Nov 7, 2016

@strk strk mentioned this pull request Nov 7, 2016
Merged
@thibaultmeyer
Copy link
Contributor

thibaultmeyer commented Nov 7, 2016 •
edited
Loading

According to the provided link, const variables must starts with lower char.

For example an unexported constant is maxLength not MaxLength or MAX_LENGTH.

@strk
Copy link
Member Author

strk commented Nov 7, 2016

Lowercase is for unexported @0xBAADF00D

@andreynering
Copy link
Contributor

LGTM

@codecov-io
Copy link

codecov-io commented Nov 7, 2016 •
edited
Loading

Current coverage is 2.24% (diff: 2.07%)

Merging #110 into master will not change coverage

@@            master      #110   diff @@
========================================
  Files           32        32          
  Lines         7521      7521          
  Methods          0         0          
  Messages         0         0          
  Branches         0         0          
========================================
  Hits           169       169          
  Misses        7335      7335          
  Partials        17        17

Powered by Codecov. Last update 5d430c9...b7bf9df

@strk strk added this to the 1.0.0 milestone Nov 7, 2016
@xinity
Copy link
Contributor

xinity commented Nov 7, 2016

LGTM

@strk strk merged commit 01a7674 into go-gitea:master Nov 7, 2016
bkcsoft
bkcsoft reviewed Nov 7, 2016
View reviewed changes
models/issue_comment.go
// Reference from a comment
COMMENT_TYPE_COMMENT_REF
CommentTypeComment_REF
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Slight mistake

models/login_source.go
LOGIN_DLDAP // 5
LoginNotype LoginType = iota
LoginPlain // 1
LoginLdap // 2
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LDAP, SMTP, PAM, DLDAP are all abbreviations and should be upper-case (Golang allows this)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also I believe it should be LoginNoType on the first one :)

models/webhook.go
@@ -28,13 +28,13 @@ var HookQueue = sync.NewUniqueQueue(setting.Webhook.QueueLength)
type HookContentType int

const (
JSON HookContentType = iota + 1
FORM
ContentTypeJson HookContentType = iota + 1
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

JSON should be upper-case

modules/auth/ldap/ldap.go
SECURITY_PROTOCOL_START_TLS
SecurityProtocolUnencrypted SecurityProtocol = iota
SecurityProtocolLdaps
SecurityProtocolStartTls
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LDAPS and TLS are upper-case

routers/repo/setting.go
@@ -22,7 +22,7 @@ const (
SETTINGS_OPTIONS base.TplName = "repo/settings/options"
COLLABORATION base.TplName = "repo/settings/collaboration"
GITHOOKS base.TplName = "repo/settings/githooks"
GITHOOK_EDIT base.TplName = "repo/settings/githook_edit"
GithookEdit base.TplName = "repo/settings/githook_edit"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why this one and not the others?

routers/repo/webhook.go
HOOK_NEW base.TplName = "repo/settings/hook_new"
ORG_HOOK_NEW base.TplName = "org/settings/hook_new"
HookNew base.TplName = "repo/settings/hook_new"
ORG_HookNew base.TplName = "org/settings/hook_new"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missed HOOKS and ORG_HookNew is wrong :)

bkcsoft
bkcsoft reviewed Nov 7, 2016
View reviewed changes
models/access.go
case "admin":
return ACCESS_MODE_ADMIN
return AccessModeAdmin
default:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not exactly related, but shouldn't this function have

case "owner":
    return AccessModeOwner

as well?

@strk
Copy link
Member Author

strk commented Nov 7, 2016 via email

@strk
Copy link
Member Author

strk commented Nov 7, 2016

On Mon, Nov 07, 2016 at 10:24:15AM -0800, bkcsoft wrote:

bkcsoft commented on this pull request.

// Reference from a comment
  • COMMENT_TYPE_COMMENT_REF
  • CommentTypeComment_REF

Slight mistake

Please review #111

@strk
Copy link
Member Author

strk commented Nov 7, 2016

On Mon, Nov 07, 2016 at 10:24:15AM -0800, bkcsoft wrote:

const (
HOOKS base.TplName = "repo/settings/hooks"

  • HOOK_NEW base.TplName = "repo/settings/hook_new"
  • ORG_HOOK_NEW base.TplName = "org/settings/hook_new"
  • HookNew base.TplName = "repo/settings/hook_new"
  • ORG_HookNew base.TplName = "org/settings/hook_new"

Missed HOOKS and ORG_HookNew is wrong :)

Also fixed in pr #111

@strk
Copy link
Member Author

strk commented Nov 7, 2016

On Mon, Nov 07, 2016 at 10:24:15AM -0800, bkcsoft wrote:

@@ -22,7 +22,7 @@ const (
SETTINGS_OPTIONS base.TplName = "repo/settings/options"
COLLABORATION base.TplName = "repo/settings/collaboration"
GITHOOKS base.TplName = "repo/settings/githooks"

  • GITHOOK_EDIT base.TplName = "repo/settings/githook_edit"
  • GithookEdit base.TplName = "repo/settings/githook_edit"

Why this one and not the others?

Fixed in #111 with [typos ee9b6e9]

@strk
Copy link
Member Author

strk commented Nov 7, 2016

On Mon, Nov 07, 2016 at 10:24:15AM -0800, bkcsoft wrote:

@@ -20,9 +20,9 @@ type SecurityProtocol int

// Note: new type must be added at the end of list to maintain compatibility.
const (

  • SECURITY_PROTOCOL_UNENCRYPTED SecurityProtocol = iota
  • SECURITY_PROTOCOL_LDAPS
  • SECURITY_PROTOCOL_START_TLS
  • SecurityProtocolUnencrypted SecurityProtocol = iota
  • SecurityProtocolLdaps
  • SecurityProtocolStartTls

LDAPS and TLS are upper-case

PR #111, [typos 10fa115]

@strk
Copy link
Member Author

strk commented Nov 7, 2016

On Mon, Nov 07, 2016 at 10:24:15AM -0800, bkcsoft wrote:

@@ -28,13 +28,13 @@ var HookQueue = sync.NewUniqueQueue(setting.Webhook.QueueLength)
type HookContentType int

const (

  • JSON HookContentType = iota + 1
  • FORM
  • ContentTypeJson HookContentType = iota + 1

JSON should be upper-case

[typos f18d87b] in pr #111

@strk
Copy link
Member Author

strk commented Nov 7, 2016

On Mon, Nov 07, 2016 at 10:24:15AM -0800, bkcsoft wrote:

@@ -28,25 +28,25 @@ type LoginType int

// Note: new type must append to the end of list to maintain compatibility.
const (

  • LOGIN_NOTYPE LoginType = iota
  • LOGIN_PLAIN // 1
  • LOGIN_LDAP // 2
  • LOGIN_SMTP // 3
  • LOGIN_PAM // 4
  • LOGIN_DLDAP // 5
  • LoginNotype LoginType = iota
  • LoginPlain // 1
  • LoginLdap // 2

Also I believe it should be LoginNoType on the first one :)

[typos 5c184e2] in pr #111

@strk
Copy link
Member Author

strk commented Nov 7, 2016

On Mon, Nov 07, 2016 at 10:26:14AM -0800, bkcsoft wrote:

bkcsoft commented on this pull request.

case "admin":

  • return ACCESS_MODE_ADMIN

  • return AccessModeAdmin
    default:

not exactly related, but shouldn't this function have

case "owner":
    return AccessModeOwner

as well?

I've also noted Owner and Admin and not sure both are used
(maybe one is just to drop - better do in a separate PR ?)

@tboerger tboerger added topic/code-linting lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. labels Nov 29, 2016
@tboerger
Copy link
Member

resolved #70

lunny pushed a commit to lunny/gitea that referenced this pull request Feb 7, 2019
@ethantkoenig @lunny
Fix tree entry parsing (go-gitea#110)
827f97a 
* Fix tree entry parsing

* nits

* populate TreeEntry.ptree
@go-gitea go-gitea locked and limited conversation to collaborators Nov 23, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@bkcsoft bkcsoft bkcsoft left review comments

Assignees
No one assigned
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. topic/code-linting
Projects
None yet
Milestone
1.0.0
Development

Successfully merging this pull request may close these issues.
7 participants
@strk @thibaultmeyer @andreynering @codecov-io @xinity @tboerger @bkcsoft