Skip to content

Rewrite raw queries #136

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.

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

Merged
merged 3 commits into from
Nov 10, 2016
Merged

Rewrite raw queries #136

merged 3 commits into from
Nov 10, 2016

Conversation

thibaultmeyer
Copy link
Contributor

@thibaultmeyer thibaultmeyer commented Nov 10, 2016

  • Use real .Count() rather than SELECT + len(array)
  • Raw "SELECT" queries now use XORM
  • Clean other XORM queries (now we can get the real line where an issue appear)
  • gofmt on all model files

@thibaultmeyer thibaultmeyer added type/enhancement An improvement of existing functionality pr/wip This PR is not ready for review labels Nov 10, 2016
@thibaultmeyer thibaultmeyer added this to the 1.1.0 milestone Nov 10, 2016
@thibaultmeyer thibaultmeyer self-assigned this Nov 10, 2016
…orm-queries

# Conflicts:
#	models/git_diff.go
#	models/issue.go
#	models/org.go
#	models/pull.go
#	models/repo.go
@thibaultmeyer thibaultmeyer changed the title [WIP] Rewrite raw queries Rewrite raw queries Nov 10, 2016
@thibaultmeyer thibaultmeyer removed in progress pr/wip This PR is not ready for review labels Nov 10, 2016
@codecov-io
Copy link

codecov-io commented Nov 10, 2016

Current coverage is 3.03% (diff: 0.00%)

Merging #136 into master will decrease coverage by 0.09%

@@            master      #136   diff @@
========================================
  Files           33        33          
  Lines         7845      8096   +251   
  Methods          0         0          
  Messages         0         0          
  Branches         0         0          
========================================
  Hits           246       246          
- Misses        7579      7830   +251   
  Partials        20        20          

Powered by Codecov. Last update 4247304...b12f2a5

Where("(`repository`.owner_id=? AND `repository`.is_private=?)", org.ID, false).
Or("team_repo.team_id IN (?)", strings.Join(base.Int64sToStrings(teamIDs), ",")).
GroupBy("`repository`.id").
Count(&Repository{})
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@@ -127,7 +127,9 @@ func GetRelease(repoID int64, tagName string) (*Release, error) {
// GetReleaseByID returns release with given ID.
func GetReleaseByID(id int64) (*Release, error) {
rel := new(Release)
has, err := x.Id(id).Get(rel)
has, err := x.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this could have be left on line, it's not that long. But it's nitpicking.

@metalmatze
Copy link
Contributor

Overall really awesome changes!

My only comment is one may lines that could have been left one-liners.

There's really no need to write

    err = x.
        Where("org_id=?", orgID).
        And("is_active=?", true).
        Find(&ws)

instead of:

    err = x.Where("org_id=?", orgID).And("is_active=?", true).Find(&ws)

It's not that much.

LGTM

@andreynering
Copy link
Contributor

My only comment is one may lines that could have been left one-liners.

Somethings fall to personal preference. I prefer it in multiple lines, but I'm OK with any style.

I'd also prefer some more spaces inside SQLs:

x.Where("org_id = ?", orgID)
// instead of
x.Where("org_id=?", orgID)

@andreynering
Copy link
Contributor

LGTM

@andreynering andreynering merged commit 52cc3fd into go-gitea:master Nov 10, 2016
@tboerger tboerger modified the milestones: 1.0.0, 1.1.0 Nov 12, 2016
@thibaultmeyer thibaultmeyer deleted the feature/rewrite-xorm-queries branch November 12, 2016 10:58
@tboerger tboerger added the lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. label Nov 29, 2016
lunny added a commit to lunny/gitea that referenced this pull request Feb 7, 2019
@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.
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. type/enhancement An improvement of existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants