Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor: wording #21

Closed
josecelano opened this issue Feb 8, 2022 · 14 comments · Fixed by #35
Closed

Refactor: wording #21

josecelano opened this issue Feb 8, 2022 · 14 comments · Fixed by #35

Comments

@josecelano
Copy link
Member

josecelano commented Feb 8, 2022

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:

  • 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

@da2ce7
Copy link
Contributor

da2ce7 commented Feb 8, 2022 via email

@josecelano
Copy link
Member Author

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:

@yeraydavidrodriguez
Copy link
Collaborator

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
Copy link
Member Author

josecelano commented Feb 9, 2022

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.

@josecelano josecelano linked a pull request Feb 9, 2022 that will close this issue
@josecelano
Copy link
Member Author

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 ✅

josecelano added a commit that referenced this issue Feb 9, 2022
josecelano added a commit that referenced this issue Feb 9, 2022
Using emojis:

New job      -> "📝🈺: "
Job finihsed -> "📝✅: "
josecelano added a commit that referenced this issue Feb 10, 2022
josecelano added a commit that referenced this issue Feb 11, 2022
The commit subject for the JobFinihsedMessage now contains the
a referecne (commit) to the commit were the job was created.

📝🈺: queue-name: job.ref.f1a69d48a01cc130a64aeac5eaf762e4ba685de7
josecelano added a commit that referenced this issue Feb 11, 2022
The commits with an standard subject without the queue refix must be be
discarded.
@da2ce7
Copy link
Contributor

da2ce7 commented Mar 4, 2022

@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?

@da2ce7 da2ce7 reopened this Mar 4, 2022
@josecelano
Copy link
Member Author

josecelano commented Mar 4, 2022

@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
Copy link
Contributor

da2ce7 commented Mar 4, 2022

@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. Add a link to the workflow execution in the library update commit chinese-ideographs-website#26

@josecelano
Copy link
Member Author

josecelano commented Mar 7, 2022

@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
Copy link
Collaborator

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
Copy link
Member Author

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:

  • Keep the first character to easily identify the queue commit.
  • Use two emojis for the message key, the first one is more descriptive and the second one is "cool".

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

  • new job: 📝*️⃣🈺
  • start job: 📝⬇️👔
  • finish job: 📝⬆️✅

@josecelano
Copy link
Member Author

@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
Copy link
Member Author

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
Copy link
Contributor

da2ce7 commented Jun 1, 2022

@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. :(

@da2ce7 da2ce7 closed this as completed Jun 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants