-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
Move license constants from /server
to /common
#32723
Conversation
Pinging @elastic/es-ui |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code LGTM! Didn't test locally.
@@ -4,15 +4,25 @@ | |||
* you may not use this file except in compliance with the Elastic License. | |||
*/ | |||
|
|||
export const LICENSE_TYPE_OSS = 'oss'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just out of curiosity, how is it possible for there to be an OSS license type if the OSS distribution ships without X-Pack?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think it is possible 🙂 I added this for the sake of completeness, and because I saw oss
included in a similar export from beats management
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 I think it might be worth checking in with them and making sure that wasn't a mistake, so we can avoid propagating it if it was.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just checked with @mattapperson and oss
was included intentionally at the time to support a "read only" mode if the user downgrades. however, plans for that support haven't really moved, so it's not necessary to declare oss
here for the time being. we can always add it when it's actually needed. thanks for calling this out, @cjcenizal, I've pushed an update that removes the oss
declaration
export const LICENSE_TYPE_BASIC = 'basic'; | ||
export const LICENSE_TYPE_STANDARD = 'standard'; | ||
export const LICENSE_TYPE_GOLD = 'gold'; | ||
export const LICENSE_TYPE_PLATINUM = 'platinum'; | ||
export const LICENSE_TYPE_TRIAL = 'trial'; | ||
|
||
export type LicenseType = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is cool! I just learned something new.
💚 Build Succeeded |
💔 Build Failed |
💚 Build Succeeded |
* Move license constants to /common * Fix export * Use constant for required license type * Remove OSS from license types
This PR moves license constants from
server
directory tocommon
and exports a new typing,LicenseType
. I used the new typing for the new snapshot and restore plugin boilerplate, but it felt disjointed from the rest of the license constants, and it felt wrong to import fromserver
for non-server usages. Moving everything tocommon
will let us use license exports everywhere.