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

Use @project on controllers, don't call method #8102

Merged
merged 1 commit into from Oct 21, 2014

Conversation

3 participants
@cirosantilli
Contributor

cirosantilli commented Oct 19, 2014

Also memoize the method through the unless @project line to ensure that other methods in
ApplicationController that rely on it can call it efficiently.

This PR makes things much more efficient: before it def project was already called as a filter before every project controller, setting @project, but many places in the controllers re-called project, which was not memoized, instead of using the instance.

@TeatroIO

This comment has been minimized.

TeatroIO commented Oct 19, 2014

I've prepared a stage. Click to open.

Use @project on controllers, don't call method
Also memoize the method to ensure that other methods in
ApplicationController that rely on it can call it efficiently.

@cirosantilli cirosantilli force-pushed the cirosantilli:use-memoized-project branch from 78bfc79 to 9e1b97a Oct 19, 2014

@cirosantilli cirosantilli changed the title from [WIP] Use @project on controllers, don't call method to Use @project on controllers, don't call method Oct 19, 2014

dzaporozhets added a commit that referenced this pull request Oct 21, 2014

Merge pull request #8102 from cirosantilli/use-memoized-project
Use @project on controllers, don't call method

@dzaporozhets dzaporozhets merged commit 5ce9f89 into gitlabhq:master Oct 21, 2014

1 check passed

default The build passed on Semaphore.
Details

@dzaporozhets dzaporozhets merged commit 9e1b97a into gitlabhq:master Oct 21, 2014

1 check passed

default The build passed on Semaphore.
Details

@cirosantilli cirosantilli deleted the cirosantilli:use-memoized-project branch Oct 21, 2014

rspeicher pushed a commit that referenced this pull request Jan 15, 2016

Merge branch 'fix-alignment-issue' into 'master'
Fix alignment issues after a fix on titles weight

Fixes #8102. Fixes #3956. This fixes an alignment regression introduced by !2422. Sorry about that! :/

## Commit title alignment fixed

![Screen_Shot_2016-01-14_at_16.10.01](/uploads/8fdc08dca379bc551f5872de910c7149/Screen_Shot_2016-01-14_at_16.10.01.png)

**The fact that the first row has a smaller height than the other rows was not introduced by !2422 (I've checked by reverting 818607f on `master`).**

## Tags name in list

![Screen_Shot_2016-01-14_at_16.10.09](/uploads/fba3978037677bb8855f374b5f38dde0/Screen_Shot_2016-01-14_at_16.10.09.png)

See merge request !2431
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment