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

add code from suggested fix to increment value of pipelineIndex #1012

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Mixmix00
Copy link

From issue #538 this is mdurrani808's suggested fix (and I think it looks good) that no one has implemented yet. It adds the number of the pipeline, starting at 0, to the nickname.

@Mixmix00 Mixmix00 requested a review from a team as a code owner November 19, 2023 23:56
@mcm001
Copy link
Contributor

mcm001 commented Nov 20, 2023

I think you might need to consider initialization order a little bit more. when does the index actually get set? Is it before or after the nickname gets initialized?

@srimanachanta
Copy link
Member

srimanachanta commented Nov 20, 2023

This would mean that the 5th pipeline made would have the 5 suffix appended to it if no name was specified which isn't the desired behavior. Logic needs to be implemented here to check the occurrence count of a current name and increment that. Pipeline names are already gauntleted to be unique when creating a new pipeline by the UI so you just need to handle the duplicating case.

@Mixmix00
Copy link
Author

This would mean that the 5th pipeline made would have the 5 suffix appended to it if no name was specified which isn't the desired behavior. Logic needs to be implemented here to check the occurrence count of a current name and increment that. Pipeline names are already gauntleted to be unique when creating a new pipeline by the UI so you just need to handle the duplicating case.

I think I have an idea, is there a method that I can use to get all of the pipeline indices that are currently being used? This way I can check to see which numbers are being used, find the max number from there, and add one to it as the new pipeline index assuming the current index isn't the integer max value.

@Mixmix00
Copy link
Author

I took a look at the code and drew a full diagram out, it seems to work fine. It leaves me wondering what the original issue was, then.

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 this pull request may close these issues.

3 participants