studio/flamenco
studio/flamenco
12
30
Fork 38
Code Issues 63 Pull Requests 5 Releases Wiki Activity

Update wording for worker maintenance guidance on Flamenco Manager Console #104227

Closed
MichaelC wants to merge 11 commits from michael-2/flamenco:vue_worker_maintenance_wording into main
pull from: michael-2/flamenco:vue_worker_maintenance_wording
merge into: studio:main

When changing the target branch, be careful to rebase the branch in your fork to match. See documentation.
Conversation 6 Commits 11 Files Changed 1 +14 -5
MichaelC commented 2023-06-30 14:34:27 +02:00
Contributor
Copy Link

Original guidance message being updated...

flamenco-adviced-message.png

Updated message texts for error and non-error scenarios...

new-message.png

new-offline-message.png

Original guidance message being updated... ![flamenco-adviced-message.png](/attachments/d41b0b40-b25e-45cb-a10d-8eb4403e9909) Updated message texts for error and non-error scenarios... ![new-message.png](/attachments/ff593cfa-259b-4dc4-aaa6-08882ffe0ad8) ![new-offline-message.png](/attachments/539fee95-5b13-4c57-9a8d-55bbf11e1668)
flamenco-adviced-message.png
29 KiB
new-message.png
27 KiB
new-offline-message.png
20 KiB
MichaelC added 1 commit 2023-06-30 14:34:29 +02:00
Francesco Siddi commented 2023-06-30 15:56:50 +02:00
Owner
Copy Link

Hi Michael, this patch improves the grammar in the first error message, but makes other messages less readable. I would simply check for "error" state and show a special message in the UI saying "workstation is in error state ", only for "error" state.

Hi Michael, this patch improves the grammar in the first error message, but makes other messages less readable. I would simply check for "error" state and show a special message in the UI saying "workstation is in error state ", only for "error" state.
MichaelC commented 2023-07-01 04:34:53 +02:00
Author
Contributor
Copy Link

Thanks for the input.

Please let me know if I'm misinterpreting... but I think from what you have said, the second message (about being awake) is new?
The vue code does already have two separate messages, one for error states and one for all other states. Maybe I shouldn't have labelled them New Messages, but rather, updated message texts.

I was not adding any more new messages. In this PR I am updating existing messages - although they are still not 100% correct grammatically they read a bit better.

Thanks for the input. Please let me know if I'm misinterpreting... but I think from what you have said, the second message (about being awake) is new? The vue code does already have two separate messages, one for error states and one for all other states. Maybe I shouldn't have labelled them New Messages, but rather, updated message texts. I was not adding any more new messages. In this PR I am updating existing messages - although they are still not 100% correct grammatically they read a bit better.
Francesco Siddi commented 2023-07-01 11:30:34 +02:00
Owner
Copy Link

What i meant was:

  • if state=='error' -> "worker is in error state"
  • else -> "worker is {{ workerData.status }}"

It's also fine as you have it now.

What i meant was: * if state=='error' -> "worker is in error state" * else -> "worker is {{ workerData.status }}" It's also fine as you have it now.
Sybren A. Stüvel requested changes 2023-07-03 10:58:01 +02:00
Sybren A. Stüvel left a comment
Owner
Copy Link

Thanks for the improvements, they certainly are that. Just a few minor things, I've mentioned those in inline comments.

Thanks for the improvements, they certainly are that. Just a few minor things, I've mentioned those in inline comments.
web/app/src/components/workers/WorkerDetails.vue Outdated
@ -119,3 +118,1 @@
<template v-if="workerData.status == 'offline'">can be safely removed.</template>
<template v-else>removing it now can cause the Worker to log errors. It
is adviced to shut down the Worker before removing it from the
<p>{{ workerData.name }} is in <span class="worker-status">{{ workerData.status }}</span> state, which means
Sybren A. Stüvel commented 2023-07-03 10:56:28 +02:00
Owner
Copy Link

I agree with @fsiddi that {name} is awake is nicer than {name} is in awake state. For the error state your improvement is certainly welcome.

I agree with @fsiddi that `{name} is awake` is nicer than `{name} is in awake state`. For the `error` state your improvement is certainly welcome.
web/app/src/components/workers/WorkerDetails.vue Outdated
@ -120,2 +118,2 @@
<template v-else>removing it now can cause the Worker to log errors. It
is adviced to shut down the Worker before removing it from the
<p>{{ workerData.name }} is in <span class="worker-status">{{ workerData.status }}</span> state, which means
<template v-if="workerData.status == 'offline'">the Worker can be safely removed.</template>
Sybren A. Stüvel commented 2023-07-03 10:57:05 +02:00
Owner
Copy Link

Given that the subject of the sentence is already the worker, I think it's clear enough to replace the worker with it.

Given that the subject of the sentence is already the worker, I think it's clear enough to replace `the worker` with `it`.
MichaelC added 1 commit 2023-07-04 15:36:17 +02:00
MichaelC added 1 commit 2023-07-04 15:38:27 +02:00
MichaelC commented 2023-07-04 15:41:37 +02:00
Author
Contributor
Copy Link

I have re-arranged the HTML a bit to be readable and not duplicate too much and factor in your feedback. I removed the word "state" entirely as it still reads fine and I had a hell of time trying to include spaces between commas etc in HTML.

I have re-arranged the HTML a bit to be readable and not duplicate too much and factor in your feedback. I removed the word "state" entirely as it still reads fine and I had a hell of time trying to include spaces between commas etc in HTML.
MichaelC requested review from Sybren A. Stüvel 2023-07-04 15:41:47 +02:00
MichaelC added 8 commits 2023-07-05 03:37:38 +02:00
MichaelC commented 2023-07-05 03:38:45 +02:00
Author
Contributor
Copy Link

@dr.sybren - Sorry for all the commits... I managed to mess up a couple of files on my main branch and have reverted them back.
Let me know if you would rather I start the PR over again.

@dr.sybren - Sorry for all the commits... I managed to mess up a couple of files on my main branch and have reverted them back. Let me know if you would rather I start the PR over again.
MichaelC commented 2023-07-05 17:32:48 +02:00
Author
Contributor
Copy Link

I have created a cleaner PR #104229 for this update.

I have created a cleaner PR https://projects.blender.org/studio/flamenco/pulls/104229 for this update.
MichaelC closed this pull request 2023-07-05 17:32:48 +02:00

Pull request closed

Please reopen this pull request to perform a merge.
Sign in to join this conversation.
Reviewers
No reviewers
Sybren A. Stüvel
Labels
Clear labels   
Good First Issue
  
Priority
High

  
Priority
Low

  
Priority
Normal

  
Status
Archived

  
Status
Confirmed

  
Status
Needs Info from Developers

  
Status
Needs Information from User

  
Status
Needs Triage

  
Status
Resolved

  
Type
Bug

  
Type
Design

  
Type
Job Type
Custom Flamenco job type

  
Type
Known Issue

  
Type
Patch

  
Type
Report

  
Type
To Do

No Label
Good First Issue
Priority
High
Priority
Low
Priority
Normal
Status
Archived
Status
Confirmed
Status
Needs Info from Developers
Status
Needs Information from User
Status
Needs Triage
Status
Resolved
Type
Bug
Type
Design
Type
Job Type
Type
Known Issue
Type
Patch
Type
Report
Type
To Do
Milestone
Clear milestone
No items
No Milestone
Assignees
Clear assignees
No Assignees
3 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: studio/flamenco#104227
No description provided.
Delete Branch "michael-2/flamenco:vue_worker_maintenance_wording"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?