Skip to content
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

Factor fork button view. #7816

Merged
merged 1 commit into from Sep 24, 2014

Conversation

4 participants
@cirosantilli
Copy link
Contributor

commented Sep 21, 2014

No description provided.

@TeatroIO

This comment has been minimized.

Copy link

commented Sep 21, 2014

I've prepared a stage. Click to open.

@Razer6 Razer6 added this to the 7.4 milestone Sep 22, 2014

%i.icon-code-fork
Fork
%span.count
= @project.forks_count

This comment has been minimized.

Copy link
@dzaporozhets

dzaporozhets Sep 22, 2014

Member

I reccomend using helpers instead of views for such elements

This comment has been minimized.

Copy link
@cirosantilli

cirosantilli Sep 22, 2014

Author Contributor

@randx I will do it. So that I follow the good rule in future PRs: how / why do you choose between them? I feel that the haml file is more readable than concatenating HTML helpers inside the helper.

This comment has been minimized.

Copy link
@dzaporozhets

dzaporozhets Sep 23, 2014

Member

@cirosantilli I use next rule for myself. If I render a control (button, link, icon) - I use helper instead of separate partial. If I render content block (text block, list item, search result) I use separate partial. Something like this. I agree that haml is more readable but I think having controls under helpers is more rails way. Ex we use link_to ... instead of = render partial: 'link_to' ....

@cirosantilli cirosantilli force-pushed the cirosantilli:factor-fork branch from 39733de to b5c0bdd Sep 23, 2014

dzaporozhets added a commit that referenced this pull request Sep 24, 2014

@dzaporozhets dzaporozhets merged commit 79a4da9 into gitlabhq:master Sep 24, 2014

1 check passed

default The build passed on Semaphore.
Details

@cirosantilli cirosantilli deleted the cirosantilli:factor-fork branch Sep 24, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.