Skip to content
Snippets Groups Projects

Moved the webhooks and services gear options to a single one called integrations

What does this MR do?

Are there points in the code the reviewer needs to double check?

Why was this MR needed?

Screenshots (if relevant)

Both webhooks and services together

Webhooks_Services

Gear button

2016-12-30_11.37.24

Does this MR meet the acceptance criteria?

What are the relevant issue numbers?

#26138 (closed)

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Grzegorz Bizon
  • @jivanvl I left a couple of notes. Great work!

  • assigned to @jivanvl

  • Jose Ivan Vargas resolved all discussions

    resolved all discussions

  • added 1 commit

    • 3f8a1d8f - Corrected code style and titles

    Compare with previous version

  • @grzesiek Improved the MR based on your observations, as for the flash solution I guess that would be a way of fixing the issue but I can't say for sure if we want more technical debt. Do you think that we should reach someone else for their input?

    Also thanks!

    Edited by Jose Ivan Vargas
  • Thanks @jivanvl + @grzesiek . Please go ahead with the best solution you believe in. From a product-design standpoint, these pages will be cleaned up later anyways.

  • @jivanvl I'm a little primed by a strong belief that we should not leave significant technical debt behind. So if you ask me, I would probably consider improving it now, but we have already at least one similar MR merged, so this wouldn't be consistent. The point here is that we should pay off this debt as soon as possible. I think that using flash messages is good as long as we are going to resolve https://gitlab.com/gitlab-org/gitlab-ce/issues/26255 soon. What do you think @rymai?

    Edited by Grzegorz Bizon
  • @jivanvl Looks good to me! :thumbsup:

  • assigned to @jivanvl

  • @jivanvl: Good for 8.16?

  • @grzesiek Let's increase our debt and pay it as soon as we have presenters, for %8.17.

    @jivanvl I had two minor remarks, otherwise, LGTM! Nice work! :heart:

  • @victorwu I think so, I just need to add the finishing touches and have it reviewed once more!

  • Thanks @jivanvl !

  • Jose Ivan Vargas resolved all discussions

    resolved all discussions

  • added 1 commit

    • 6b684449 - Removed the index action from both the projects hook and services controllers

    Compare with previous version

  • added 1 commit

    • 6b684449 - Removed the index action from both the projects hook and services controllers

    Compare with previous version

  • Jose Ivan Vargas added 851 commits

    added 851 commits

    • 6b684449...61b6643e - 845 commits from branch master
    • 9f0d7945 - Moved the webhooks and services gear options to a single one called integrations
    • 1ee4f986 - Fixed spinach feature tests for the services and hooks controllers
    • bc7c6c68 - Fixed more tests
    • 373411d1 - Corrected code style and titles
    • 0af99433 - Removed the index action from both the projects hook and services controllers
    • 7b3ea49a - Fixed tests and a rubocop linter

    Compare with previous version

  • @rymai Improved the MR based on your comments, can you take a look once more when you get a chance?

    Thanks :thumbsup:

  • @jivanvl LGTM but please open a MR against EE as well (see https://gitlab.com/gitlab-org/gitlab-ce/builds/8916353)!

  • Rémy Coutable added ~149423 label

    added ~149423 label

  • Once done and the ee_compat_check build pass, I will merge it.

  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Please register or sign in to reply
    Loading