Deprecate ShapeIndex in favor of Model#209
Merged
Conversation
JordonPhillips
requested changes
Nov 22, 2019
...tware/amazon/smithy/aws/traits/clientendpointdiscovery/ClientEndpointDiscoveryTraitTest.java
Outdated
Show resolved
Hide resolved
smithy-model/src/main/java/software/amazon/smithy/model/Model.java
Outdated
Show resolved
Hide resolved
smithy-model/src/main/java/software/amazon/smithy/model/transform/ReplaceShapes.java
Outdated
Show resolved
Hide resolved
mtdowling
force-pushed
the
deprecate-shape-index
branch
from
757437a to
9481be1
Compare
kstich
requested changes
Nov 25, 2019
smithy-build/src/main/java/software/amazon/smithy/build/PluginContext.java
Outdated
Show resolved
Hide resolved
smithy-model/src/main/java/software/amazon/smithy/model/transform/ModelTransformer.java
Outdated
Show resolved
Hide resolved
smithy-build/src/main/java/software/amazon/smithy/build/PluginContext.java
Outdated
Show resolved
Hide resolved
mtdowling
force-pushed
the
deprecate-shape-index
branch
from
9481be1 to
767e9f0
Compare
Model. Separating ShapeIndex from Model made building models much more verbose, interacting with shapes in a model more verbose, updating models verbose, and caused an awkward API if you need to access things like knowledge indexes but only have access to a ShapeIndex (knowledge indexes are only on models). This commit first deprecates all ShapeIndex APIs and provides alternatives that are to be used instead. In some cases, the alternative, Model based APIs, still call into the deprecated ShapeIndex APIs. This is because a Model always has a ShapeIndex but a ShapeIndex is not a model, and it cuts down on code duplication. In the next version bump (likely 0.10.0), we will remove the ShapeIndex APIs altogether.
mtdowling
force-pushed
the
deprecate-shape-index
branch
from
767e9f0 to
9d9831a
Compare
JordonPhillips
approved these changes
Nov 25, 2019
kstich
approved these changes
Nov 25, 2019
2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
ShapeIndex is not adding any value as a separate abstraction from
Model. Separating ShapeIndex from Model made building models much more
verbose, interacting with shapes in a model more verbose, updating
models verbose, and caused an awkward API if you need to access things
like knowledge indexes but only have access to a ShapeIndex (knowledge
indexes are only on models).
This commit first deprecates all ShapeIndex APIs and provides
alternatives that are to be used instead. In some cases, the
alternative, Model based APIs, still call into the deprecated ShapeIndex
APIs. This is because a Model always has a ShapeIndex but a ShapeIndex
is not a model, and it cuts down on code duplication. In the next
version bump (likely 0.10.0), we will remove the ShapeIndex APIs
altogether.
Issue #, if available:
Description of changes:
Addresses #208
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.