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

[refactor] Factor out commit list from compare and new MR. #7657

Merged
merged 1 commit into from Sep 22, 2014

Conversation

4 participants
@cirosantilli
Contributor

cirosantilli commented Aug 31, 2014

The exact same piece of code was copied twice.

Besides those, there is also a third very similar copy which has started to diverge for the merge requests show view at:

. It has diverged as it now:

  • has a list icon
  • hides to 8 items on page load

I propose all of those 3 cases be made the same. If not, at least we should further refactor part of the MR show with the other 2 occurrences.

Just to show clearly what is the object in question:

screenshot from 2014-08-31 13 07 27 commit list

@TeatroIO

This comment has been minimized.

TeatroIO commented Aug 31, 2014

I've prepared a stage. Click to open.

@cirosantilli cirosantilli changed the title from Factor out commit list from compare and new MR. to [refactor] Factor out commit list from compare and new MR. Aug 31, 2014

@cirosantilli

This comment has been minimized.

Contributor

cirosantilli commented Sep 1, 2014

Current errors appear unrelated: I get a lot of:

No such file or directory - /home/travis/build/gitlabhq/gitlabhq/tmp/tests/repositories/root/testme.git/
@Razer6

This comment has been minimized.

Member

Razer6 commented Sep 13, 2014

@cirosantilli Could you rebase?

@cirosantilli cirosantilli force-pushed the cirosantilli:factor-commit-list branch from 9409fb2 to 1da0c0b Sep 14, 2014

@cirosantilli

This comment has been minimized.

Contributor

cirosantilli commented Sep 14, 2014

Rebased.

@Razer6

This comment has been minimized.

Member

Razer6 commented Sep 15, 2014

@randx Looks good!

@dzaporozhets

This comment has been minimized.

Member

dzaporozhets commented Sep 15, 2014

Cool. Today is code freeze so candidate for 7.4

@dzaporozhets dzaporozhets self-assigned this Sep 15, 2014

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

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

Merge pull request #7657 from cirosantilli/factor-commit-list
[refactor] Factor out commit list from compare and new MR.

@dzaporozhets dzaporozhets merged commit e54fdee into gitlabhq:master Sep 22, 2014

@dzaporozhets

This comment has been minimized.

Member

dzaporozhets commented Sep 22, 2014

@cirosantilli thank you!

@cirosantilli

This comment has been minimized.

Contributor

cirosantilli commented Sep 22, 2014

good for gitlab --> good for me too =)

@cirosantilli cirosantilli deleted the cirosantilli:factor-commit-list branch Sep 22, 2014

tigefa4u pushed a commit to tigefa4u/gitlabhq that referenced this pull request Nov 6, 2018

Nick Thomas
Merge branch 'ce-52112-fix-review-apps-cleanup' into 'master'
Fix review apps cleanup

Closes gitlabhq#7657 and gitlab-ce#52112

See merge request gitlab-org/gitlab-ee!7772
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment