nautilus-cyberneering / git-queue

A GitHub Action that implements a job queue with a concurrency lock by using Git empty commits
https://nautilus-cyberneering.github.io/git-queue/
MIT License
14 stars 3 forks source link

Refactor: wording #21

Closed josecelano closed 2 years ago

josecelano commented 2 years ago

In the original repo, we started a discussion about wording:

  1. Commit message subject
  2. Code: message names

IMPORTANT: allowing more than one job is still not implemented. We can implement this issue without adding the job.{#JOB_POSITION}/{#PENDING_JOBS}: part of the subject.

Commit subject

New job:

πŸ“πŸˆΊ: {QUEUE_NAME}: job.{JOB_NUMBER}
πŸ“πŸˆΊ: update_artwork: job.24

Start job:

πŸ“πŸ‘”: {QUEUE_NAME}: job.{#JOB_POSITION}/{#PENDING_JOBS}: job.ref.{JOB_CREATION_COMMIT_HASH}
πŸ“πŸ‘”: update_artwork: job.15/30: job.ref.1e31b549c630f806961a291b4e3d4a1471f37490

Finish job

πŸ“πŸ‘•: {QUEUE_NAME}: job.{#JOB_POSITION}/{PENDING_JOBS}: job.ref.{JOB_CREATION_COMMIT_HASH}
πŸ“πŸ‘•: update_artwork: job.15/30: job.ref.1e31b549c630f806961a291b4e3d4a1471f37490

Variables:

Examples:

Job 24 was created:

πŸ“πŸˆΊ: update_artwork: job.24

The job created in the commit 1e31b549c630f806961a291b4e3d4a1471f37490 (24) has started and it's currently the 15th job in the 30 not finished jobs.

πŸ“πŸ‘”: update_artwork: job.15/30: job.ref.1e31b549c630f806961a291b4e3d4a1471f37490

The previous jobs has finished:

πŸ“πŸ‘•: update_artwork: job.15/30: job.ref.1e31b549c630f806961a291b4e3d4a1471f37490

Notes:

Message names

In the source code we have two types of messages:

We have to rename them to:

We will add the new message JobStartedMessage in a different issue.

please @da2ce7 could you review this issue? especially the #PENDING_JOBS and #JOB_POSITION variables. I do not know if I got them right.

cc @yeraydavidrodriguez

da2ce7 commented 2 years ago

Hello Jose,

Thank You for the good Write-Up.

Firstly, the job complete Emoji might be better to have the β˜‘οΈ Checkmark, it is more visually distinct from the casual shirt.

Secondly,

JOB_POSITION and #PENDING_JOBS

When creating a new job, it will always have the highest job number in that particular queue.

So I think that we only need the two variables:

JOB_NUMBER and #JOB_ACTIVE

Then visually:

Created Job 30.

Starting/Finishing Job 15 (of 30).

:)

Cam.

Von meinem iPhone gesendet

Am 8/2/2022 um 15:36 schrieb Jose Celano @.***>:

ο»Ώ In the original repo, we started a discussion about wording:

Commit message subject Code: message names Commit subject

New job:

πŸ“πŸˆΊ: {QUEUE_NAME}: job.{JOB_NUMBER} πŸ“πŸˆΊ: update_artwork: job.24 Start job:

πŸ“πŸ‘”: {QUEUE_NAME}: job.{#JOB_POSITION}/{#PENDING_JOBS}: job.ref.{JOB_CREATION_COMMIT_HASH} πŸ“πŸ‘”: update_artwork: job.15/30: job.ref.1e31b549c630f806961a291b4e3d4a1471f37490 Finish job

πŸ“πŸ‘•: {QUEUE_NAME}: job.{#JOB_POSITION}/{PENDING_JOBS}: job.ref.{JOB_CREATION_COMMIT_HASH} πŸ“πŸ‘•: update_artwork: job.15/30: job.ref.1e31b549c630f806961a291b4e3d4a1471f37490 Variables:

JOB_NUMBER is a job (not message) auto-incremental ID. QUEUE_NAME is the name of the queue. Special characters or white spaces are replaced by -. Max. length 30 after formatting. JOB_CREATION_COMMIT_HASH is the has of the commit where the job was created.

PENDING_JOBS is the number of not finished jobs in the queue.

JOB_POSITION in the not finished jobs.

Examples:

Job 24 was created:

πŸ“πŸˆΊ: update_artwork: job.24 The job created in the commit 1e31b549c630f806961a291b4e3d4a1471f37490 (24) has started and it's currently the 15th job in the 30 not finished jobs.

πŸ“πŸ‘”: update_artwork: job.15/30: job.ref.1e31b549c630f806961a291b4e3d4a1471f37490 The previous jobs has finished:

πŸ“πŸ‘•: update_artwork: job.15/30: job.ref.1e31b549c630f806961a291b4e3d4a1471f37490 Notes:

JOB_POSITION and #PENDING_JOBS (job.15/30 in the example) are always the same for the same job because the commits are merged at the same time.

Message names

In the source code we have two types of messages:

CreateJobMessage MarkJobAsDoneMessage We have to rename them to:

NewJobMessage JobFinishedMessage We will add the new message JobStartedMessage in a different issue.

please @da2ce7 could you review this issue? especially the #PENDING_JOBS and #JOB_POSITION variables. I do not know if I got them right.

cc @yeraydavidrodriguez

β€” Reply to this email directly, view it on GitHub, or unsubscribe. Triage notifications on the go with GitHub Mobile for iOS or Android. You are receiving this because you were mentioned.

josecelano commented 2 years ago

hi @da2ce7 OK, them the examples would look like this:

πŸ“πŸˆΊ: update_artwork: job.24
πŸ“πŸ‘”: update_artwork: job.15/30: job.ref.1e31b549c630f806961a291b4e3d4a1471f37490
πŸ“βœ…: update_artwork: job.15/30: job.ref.1e31b549c630f806961a291b4e3d4a1471f37490

being 15 the # of the job #JOB_NUMBER and 30 the # of pending jobs in the queue when the message is added to the queue (we also consider a pending job the job we are finishing). SO job.15/30 means: job 15 and there are 30 pending jobs including the 15 one.

Regarding the emoji for the check, you have used β˜‘ but I think βœ… fits better with the other icons style.

There are three checks icons:

  1. βœ”
  2. βœ…
  3. β˜‘
yeraydavidrodriguez commented 2 years ago

I suggest using any of the "boxed" checks. Are more consistent with the other emojis, and also is safer for both light and dark themes. Regarding the color, green is more semantic than blue (green = everything went well), so I would also suggest using it.

josecelano commented 2 years ago

This is how I was seeing the icons while writting the comment:

image so I agree with you @yeraydavidrodriguez I would use the number 3 which has these two images depending on whether you using it as plain text or inside a "code" block or a list.

βœ…

β˜‘

  1. β˜‘
josecelano commented 2 years ago

I've found this:

image

Direct text:

βœ”οΈ β˜‘οΈ βœ…

Inside code block:

βœ”οΈ
β˜‘οΈ
βœ…

And this is how they look with VSCode:

image

The second one does not look nice in some cases. So I'm going to use the third one βœ…

da2ce7 commented 2 years ago

@josecelano @yeraydavidrodriguez I would like to reopen this issue.

The current implementation looks like this:

Screenshot 2022-03-04 at 15 11 26

Here is a mock-up of what that could be more clear:

Screenshot 2022-03-04 at 15 42 45
  1. Use the Up and Down arrows to visually show where the job's commits are.
  2. The Finishing Commit should reference the starting Commit, not the Job Specification.
  3. Slight re-working of the wording.

What do you think?

josecelano commented 2 years ago

@josecelano @yeraydavidrodriguez I would like to reopen this issue.

The current implementation looks like this:

Screenshot 2022-03-04 at 15 11 26

Here is a mock-up of what that could be more clear:

Screenshot 2022-03-04 at 15 42 45
  1. Use the Up and Down arrows to visually show where the job's commits are.
  2. The Finishing Commit should reference the starting Commit, not the Job Specification.
  3. Slight re-working of the wording.

What do you think?

  1. OK, but we were using the first char to identify queue commits (commit subject prefix). I suppose it's not a problem if we have more than one possible prefix. Al alternative could be keep the prefix and use 1 or 2 icons for the job message key (that's how I call the icon for the job).
  2. OK. So the starting commit references the creation commit, and the finishing commit the starting one.
  3. OK. I also find it useful to add the workflow id in the message metadata once we implement that feature. That makes it very easy to debug.

EDIT: by the way, I do not like it too much the delimiter we are using (":"). It seems we are using emojis for all except for that. Specially the first one after the job message key.

da2ce7 commented 2 years ago

@josecelano

  1. We use the first Character to Identify the Queue Commits. Maybe we should then remove the "πŸ“" character. Then we should have a workflow that forbids any other commit to start with the "🈺", "⬆️" and "⬇️" emojis.

  2. Yes, we can put the workflow reference into both the Job Allocation and Job Finished commits. https://github.com/Nautilus-Cyberneering/chinese-ideographs-website/issues/26

josecelano commented 2 years ago

@josecelano

  1. We use the first Character to Identify the Queue Commits. Maybe we should then remove the "memo" character. Then we should have a workflow that forbids any other commit to start with the "u55b6", "arrow_up" and "arrow_down" emojis.
  2. Yes, we can put the workflow reference into both the Job Allocation and Job Finished commits. Add a link to the workflow execution in the library update commitΒ chinese-ideographs-website#26

In other to implement this change I think it makes sense to totally remove the queue commit prefix. The first symbol in the commit subject is the message key. So we do not have any character to identify all commits. We only check if the first character is a valid message key.

export class NewJobMessage extends Message {
  constructor(payload: string) {
    super(payload, nullCommitHash())
  }

  getKey(): MessageKey {
    return new MessageKey('🈺')
  }
}

export class JobFinishedMessage extends Message {
  getKey(): MessageKey {
    return new MessageKey('β¬‡οΈβœ…')
  }
}

export class JobStartedMessage extends Message {
  getKey(): MessageKey {
    return new MessageKey('β¬†πŸ‘”')
  }
}

We also have to change the method to identify a commit subject.

yeraydavidrodriguez commented 2 years ago

After thinking a lot about this issue, and having read all the discussion and the arguments I would propose another alternative.

Although at first I thought that removing the prefix would be a good idea, now I think otherwise. I think we should have a two-emojis prefix. The reason is that the fixed first character would be extremely useful to grep the gitlog or do a manual search.

And for the second emoji, I like the ideas of the arrows. They are very expressive, cross-cultural and all fonts paint them coherently. It also works as a closure of the job.

I only have some concerns about the new job emoji (the Kanji). Although we all agree that would highlight the new job commit in a very prominent way, and we all think Kanjis are beautiful, I also think we are sacrificing expressivity and legibility. Few users would know the reason behind this choice. I would use, from the same emojis section of the arrows, the asterisk symbol, that resembles the star associated with mnay "new" action buttons.

I would propose, then, this prefix scheme:

new job:    πŸ“ *️⃣
start job:  πŸ“ ⬇️
finish job: πŸ“ ⬆️

What do you think, @josecelano @da2ce7 @cgbosse ?

josecelano commented 2 years ago

After thinking a lot about this issue, and having read all the discussion and the arguments I would propose another alternative.

Although at first I thought that removing the prefix would be a good idea, now I think otherwise. I think we should have a two-emojis prefix. The reason is that the fixed first character would be extremely useful to grep the gitlog or do a manual search.

And for the second emoji, I like the ideas of the arrows. They are very expressive, cross-cultural and all fonts paint them coherently. It also works as a closure of the job.

I only have some concerns about the new job emoji (the Kanji). Although we all agree that would highlight the new job commit in a very prominent way, and we all think Kanjis are beautiful, I also think we are sacrificing expressivity and legibility. Few users would know the reason behind this choice. I would use, from the same emojis section of the arrows, the asterisk symbol, that resembles the star associated with mnay "new" action buttons.

I would propose, then, this prefix scheme:

new job:    πŸ“ *️⃣
start job:  πŸ“ ⬇️
finish job: πŸ“ ⬆️

What do you think, @josecelano @da2ce7 @cgbosse ?

Maybe we could:

Regarding the Kanji I agree that the asterisc could be more explicit, but probably the "+" is the most used.

josecelano commented 2 years ago

@da2ce7 any suggestions? I think there are 2 pending questions:

1- Should we use a fix prefix or totally remove it? 2- Should change the "new job" icon

We need to move on with this issue, I think my latest proposal has a little bit of all proposals and it's symmetric (something developers usually like) :-). If we do not agree on a solution I would say we can go with Cameron's latest proposal since he was the ideator because I do not see any major inconvenience in any of them. It's more a UX issue.

josecelano commented 2 years ago

hi @da2ce7 should we implement this change? I would say we can close the issue and leave it as it's it because that's going to change a lot in version 2.0.

da2ce7 commented 2 years ago

@josecelano I agree and have chosen to close this issue. The next version is going to look very different, so this thread isn't so relevant anymore. :(