Arvados

21413-retried-badge @ 4ce5db8508377bd5d4825cc263957bcab1cd2ee1

developer-run-tests: #4397

21413-retried-badge @ ef508246d95c9fe90579adfa8bd22e6cc1eaafc4

developer-run-tests: #4400

• All agreed upon points are implemented / addressed.
  • Now gives better feedback that a workflow step was resubmitted automatically
• Anything not implemented (discovered or discussed during work) has a follow-up story.
  • n/a
• Code is tested and passing, both automated and manual, what manual testing was done is described
  • passes existing tests, I did not add any new tests.
• Documentation has been updated.
  • n/a
• Behaves appropriately at the intended scale (describe intended scale).
  • no changes to scale
• Considered backwards and forwards compatibility issues between client and server.
  • Only has an effect when an newer arvados-cwl-runner that sets "arv:failed_container_resubmitted" property is used
• Follows our coding standards and GUI style guidelines.
  • yes

• I think your editor is misconfigured and using TABs for identation instead of spaces: there's some weird indentation at least in process.ts
• File services/workbench2/src/views/process-panel/process-details-attributes.tsx
  • Line 115: Instead of using the literal string, we could use the enum like in line 118 for consistency.
  • Weird mix of tab and space indentation going on here too.
• I believe you should be able to easily add a test case for this new badge in services/workbench2/src/views-components/data-explorer/renderers.test.tsx

Lucas Di Pentima wrote in #note-23:

• I think your editor is misconfigured and using TABs for identation instead of spaces: there's some weird indentation at least in process.ts

Yea, I had a hook to automatically fix tabs-to-spaces but it wasn't hooked to the right mode. Fixed.

• File services/workbench2/src/views/process-panel/process-details-attributes.tsx
  • Line 115: Instead of using the literal string, we could use the enum like in line 118 for consistency.

Fixed.

• Weird mix of tab and space indentation going on here too.

Fixed.

• I believe you should be able to easily add a test case for this new badge in services/workbench2/src/views-components/data-explorer/renderers.test.tsx

I was actually avoiding adding unit tests on purpose to avoid having merge conflicts with #21720

We can keep the ticket open and add tests when the big wb2 branch lands.

Some missing comments from the previous review pass:

• At services/workbench2/src/views/process-panel/process-details-attributes.tsx line 118, it seems that if getResourceUrl() fails, we build an empty link. I think it would be better to not render the link in that case, instead.
• Our workbench color guidelines suggest that colors should have some minimal contrast between each other. Not sure if this orange color complies with the suggestion, but wanted to point it out just in case.

Apart from the above observations, it LGTM.

Lucas Di Pentima wrote in #note-25:

Some missing comments from the previous review pass:

• At services/workbench2/src/views/process-panel/process-details-attributes.tsx line 118, it seems that if getResourceUrl() fails, we build an empty link. I think it would be better to not render the link in that case, instead.

Now won't render the notice if getResourceUrl returns undefined.

• Our workbench color guidelines suggest that colors should have some minimal contrast between each other. Not sure if this orange color complies with the suggestion, but wanted to point it out just in case.

Now uses a "dark orange" which has a 4.5 contrast ratio with white text.

Apart from the above observations, it LGTM.

21413-retried-badge @ abdc66377cccd1e87617ca333d0cd1d2883f1591

developer-run-tests: #4404

LGTM, thanks!