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

Identifiers with configurable prefixes #727

Merged
merged 2 commits into from
Dec 2, 2022
Merged

Conversation

tasdomas
Copy link
Contributor

@tasdomas tasdomas commented Dec 1, 2022

This is needed to be able to differentiate between tasks allocated using leo server and tpi/leo cli.

@tasdomas tasdomas temporarily deployed to automatic December 1, 2022 14:38 Inactive
@tasdomas tasdomas temporarily deployed to automatic December 1, 2022 14:38 Inactive
@tasdomas tasdomas temporarily deployed to automatic December 1, 2022 14:38 Inactive
@tasdomas tasdomas temporarily deployed to automatic December 1, 2022 14:38 Inactive
@tasdomas tasdomas temporarily deployed to automatic December 1, 2022 14:39 Inactive
@0x2b3bfa0
Copy link
Member

Also useful for #726 and cml- prefixes, but I would argue that there is no need to differentiate leo-server from the leo command-line tool.

Copy link
Member

@0x2b3bfa0 0x2b3bfa0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why enforce the 3 character length requirement? Alternatively, can we just have some const prefix values to choose from? E.g.

type prefix string

const (
    prefixTPI prefix = "tpi"
    prefixLEO prefix = "leo"
    prefixCML prefix = "cml"
)

@0x2b3bfa0 0x2b3bfa0 temporarily deployed to automatic December 1, 2022 15:04 Inactive
@0x2b3bfa0 0x2b3bfa0 temporarily deployed to automatic December 1, 2022 15:04 Inactive
@0x2b3bfa0 0x2b3bfa0 temporarily deployed to automatic December 1, 2022 15:04 Inactive
@0x2b3bfa0 0x2b3bfa0 temporarily deployed to automatic December 1, 2022 15:04 Inactive
@0x2b3bfa0 0x2b3bfa0 temporarily deployed to automatic December 1, 2022 15:04 Inactive
@tasdomas
Copy link
Contributor Author

tasdomas commented Dec 2, 2022

Why enforce the 3 character length requirement?

To avoid having to recalculate the lengths of other fields in the identifier.

@tasdomas
Copy link
Contributor Author

tasdomas commented Dec 2, 2022

Why enforce the 3 character length requirement? Alternatively, can we just have some const prefix values to choose from? E.g.

type prefix string

const (
    prefixTPI prefix = "tpi"
    prefixLEO prefix = "leo"
    prefixCML prefix = "cml"
)

I don't see a big difference - keeping it a simple string seems slightly more extendable.

@tasdomas tasdomas requested review from 0x2b3bfa0 and a team December 2, 2022 12:05
Copy link
Member

@0x2b3bfa0 0x2b3bfa0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To my mind, the “flexibility” provided by acepting arbitrary prefixes is a bit of a footcannon, but... 🤷🏼‍♂️

@0x2b3bfa0 0x2b3bfa0 changed the title Identifiers with configurable prefixes. Identifiers with configurable prefixes Dec 2, 2022
@tasdomas tasdomas merged commit a24fe43 into master Dec 2, 2022
@tasdomas tasdomas deleted the d033-identifier-prefix branch December 2, 2022 13:55
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.

2 participants