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

Feature: Optional name for activity descriptions #127

Open
carlaKC opened this issue Oct 5, 2023 · 11 comments
Open

Feature: Optional name for activity descriptions #127

carlaKC opened this issue Oct 5, 2023 · 11 comments
Assignees
Labels
feature New feature or request good first issue Good for newcomers help wanted Extra attention is needed Medium Priority
Milestone

Comments

@carlaKC
Copy link
Contributor

carlaKC commented Oct 5, 2023

Add an optional "name" field on activity descriptions that can be used as a logging prefix so that it's easy to relate logs to their corresponding activity description.

@carlaKC carlaKC added feature New feature or request good first issue Good for newcomers help wanted Extra attention is needed Medium Priority labels Oct 5, 2023
@carlaKC carlaKC added this to the V1 Minor Improvements milestone Oct 11, 2023
@Anyitechs
Copy link

Hi @carlaKC, I'll like to work on this if it's still available.

@carlaKC
Copy link
Contributor Author

carlaKC commented Feb 27, 2024

Sure! Since this is a user-facing issue, please post a brief design proposal on how you'd like to update the simulation file:

  • Changes we'd make to the json to add to the label for actvities
  • What we're going to do when we have no labels (use index in json?)
  • How we'll handle validation: do we allow some activities with labels and some without?

@Anyitechs
Copy link

Great! Thank you. I'll come up with a design proposal and share with you here.

@ikeogu
Copy link

ikeogu commented Mar 2, 2024

hello everyone, this is great, but can I come up with a design proposal to work on this? I want to contribute.

@Anyitechs
Copy link

hello everyone, this is great, but can I come up with a design proposal to work on this? I want to contribute.

I'm working on the proposal already. Had some blockers setting up project locally earlier but fixed that already.

@ikeogu
Copy link

ikeogu commented Mar 2, 2024

ohh!, its alright!

@ikeogu
Copy link

ikeogu commented Mar 2, 2024

Let me look out for another issue to contribute to.

@Anyitechs
Copy link

Anyitechs commented Mar 6, 2024

Sure! Since this is a user-facing issue, please post a brief design proposal on how you'd like to update the simulation file:

Thank you. I'm sorry I didn't come back to this immediately. I had to look at the test cases and made some draft changes to understand the moving parts first.

  • Changes we'd make to the json to add to the label for actvities

We'll add a new field to the activity description. I'm thinking we can label it activity_name or just name. Then update the following:

  • ActivityParser struct
  • ActivityDefinition struct
  • validated_activities
  • Test cases and lastly,
  • Use the activity "name" on logging.

Let me know if I'm missing something.

  • What we're going to do when we have no labels (use index in json?)

We can use the source and destination node aliases together as the label if the aliases were provided in json and not the node ID. Otherwise we use the index. For example, Alice to Bob activity can be "Alice->Bob" as the label, or "Activity 0" (where "0" is the index of the activity) as the label if the node ID was used.

  • How we'll handle validation: do we allow some activities with labels and some without?

I'm not sure what the best approach is. I'm thinking we can allow activities with or without labels, but if an activity doesn't have a label, then we can make up a label from their alias or activity index in json.

Let me know what your thoughts are. @carlaKC

@carlaKC
Copy link
Contributor Author

carlaKC commented Mar 6, 2024

We can use the source and destination node aliases together as the label if the aliases were provided in json and not the node ID. Otherwise we use the index. For example, Alice to Bob activity can be "Alice->Bob" as the label, or "Activity 0" (where "0" is the index of the activity) as the label if the node ID was used.

I don't think that we should have multiple ways of doing this depending on what's there, I think it'll be confusing. Would be happy to just use the indexes to start with. We already log the "Alice->Bob" info as part of the description, so I think that way we won't be duplicating anything.

I'm thinking we can allow activities with or without labels, but if an activity doesn't have a label, then we can make up a label from their alias or activity index in json.

SGTM!

Feel free to go ahead and open up a PR, please do open it up early/in draft for a concept ACK so that we can get an early idea of what it'll look like!

@Anyitechs
Copy link

We can use the source and destination node aliases together as the label if the aliases were provided in json and not the node ID. Otherwise we use the index. For example, Alice to Bob activity can be "Alice->Bob" as the label, or "Activity 0" (where "0" is the index of the activity) as the label if the node ID was used.

I don't think that we should have multiple ways of doing this depending on what's there, I think it'll be confusing. Would be happy to just use the indexes to start with. We already log the "Alice->Bob" info as part of the description, so I think that way we won't be duplicating anything.

SGTM! I'll update to use indexes.

I'm thinking we can allow activities with or without labels, but if an activity doesn't have a label, then we can make up a label from their alias or activity index in json.

SGTM!

Thank you.

Feel free to go ahead and open up a PR, please do open it up early/in draft for a concept ACK so that we can get an early idea of what it'll look like!

Sure! Thank you so much for your response. I'll go ahead to complete this and open a PR immediately for review.

@Anyitechs
Copy link

Hi @carlaKC , I opened a PR draft here. Let me know what your thoughts are when you get to take a look at it.

@carlaKC carlaKC modified the milestones: V1 Minor Improvements, V3 Mar 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request good first issue Good for newcomers help wanted Extra attention is needed Medium Priority
Projects
None yet
Development

No branches or pull requests

3 participants