[ignore] add support for version setting to class from meta and override (DCNE-636) #1439
Conversation
akinross
requested review from
anvitha-jain,
gmicol,
lhercot,
sajagana,
samiib and
shrsr
as code owners
github-actions
bot
changed the title
akinross
force-pushed
the
generator_redesign
branch
from
5d5854e to
0849fe1
Compare
gen/utils/data/class.go
Outdated
| // A dash at the end of a range (ex. 4.2(7f)-) indicates that the class is supported from the first version to the latest version. | ||
| Versions []VersionRange | ||
| // Parsed from the "versions" field in the meta file (e.g., "1.0(1e)-", "4.2(7f)-4.2(7w),5.2(1g)-"). | ||
| // TODO: Add DeprecatedVersions field when meta file exposes deprecation information. |
There was a problem hiding this comment.
Why not already support DeprecatedVersions from override files so we can already get started using it?
There was a problem hiding this comment.
Initially removed because it is not supported from meta, and not sure how we would implement it. Kind of following YAGNI principle, which is not always the correct approach if you foresee it will be implemented. But think especially considering it being a completely isolated part it could have easily been added later.
After our discussion put it back where it only allows it being set from definition. We should have a follow up discussion on development documentation that documents these things properly. Issue is already created for tracking this.
gen/utils/data/version.go
Outdated
| return versionRanges, nil | ||
| } | ||
|
|
||
| func parseVersionRange(metaVersionRange string) (*VersionRange, error) { |
There was a problem hiding this comment.
Shouldn't this be NewVersionRange from the above NewVersions?
There was a problem hiding this comment.
Changed it from consistency point of view.
Not sure it was really needed and if the current split is actually then the "right" approach. We could explore changing all these structs to leverage builder pattern or exposing the attributes as input but in the scope of the generator I find this overkill. I think for our generator usecase the current split provides enough flexibility without adding to much complexity.
…ween the functions
akinross
force-pushed
the
generator_redesign
branch
from
7e9356a to
04b2e20
Compare
4 participants
No description provided.