[go: up one dir, main page]

Skip to content

Add help link for MRs that need a successful pipeline

As part of the epic to improve the MR widget text (&6436) I've come to this message, which I need help from a frontend engineer to improve:

@pedroms has suggested instead:

Amy's research

I grepped for part of the string (grep -rl "pipeline must run and be successful" . | grep -v "locale/" | grep -v "tmp/") and got these results:

./app/assets/javascripts/vue_merge_request_widget/mixins/ready_to_merge.js
./coverage-frontend/lcov-report/app/assets/javascripts/vue_merge_request_widget/mixins/ready_to_merge.js.html
./coverage-frontend/lcov-report/ee/app/assets/javascripts/vue_merge_request_widget/mixins/ready_to_merge.js.html
./ee/app/assets/javascripts/vue_merge_request_widget/mixins/ready_to_merge.js
./doc/ci/troubleshooting.md

When I looked at the first file, I saw this string was being set as a constant: PIPELINE_MUST_SUCCEED_CONFLICT_TEXT. That approach made me ask "is it being used elsewhere?" This grep (grep -rl PIPELINE_MUST_SUCCEED_CONFLICT_TEXT . | grep -v "locale/") turned up an extra test to modify:

./app/assets/javascripts/vue_merge_request_widget/mixins/ready_to_merge.js
./coverage-frontend/lcov-report/app/assets/javascripts/vue_merge_request_widget/mixins/ready_to_merge.js.html
./coverage-frontend/lcov-report/ee/app/assets/javascripts/vue_merge_request_widget/mixins/ready_to_merge.js.html
./ee/app/assets/javascripts/vue_merge_request_widget/mixins/ready_to_merge.js
./ee/spec/frontend/vue_mr_widget/components/states/mr_widget_ready_to_merge_spec.js

But where was the link coming from? I searched for part of the string of the link, and turned up app/views/projects/merge_requests/_widget.html.haml with a big pile of links. The part we care about is line 14, window.gl.mrWidgetData.pipeline_must_succeed_docs_path.

Amy's concern

Pedro's suggested revision would require changes to these files:

  1. ./app/assets/javascripts/vue_merge_request_widget/mixins/ready_to_merge.js
  2. ./ee/app/assets/javascripts/vue_merge_request_widget/mixins/ready_to_merge.js
  3. ./ee/spec/frontend/vue_mr_widget/components/states/mr_widget_ready_to_merge_spec.js
  4. app/views/projects/merge_requests/_widget.html.haml
  5. ./doc/ci/troubleshooting.md

Given how the links are handled separately in _widget.html.haml, is it even possible to do the string with the link in the middle, as Pedro suggested? I suspect not. However, I'd really appreciate having help from a FE to figure out what I can and can't do with this string.

cc @andr3