-
Notifications
You must be signed in to change notification settings - Fork 1k
Rename [[dependencies]] to [[constraint]] #538
Conversation
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.
looks pretty good, thank you!!! 🎉
one wording nit, then a request for a bit more work 😄
txn_writer.go
Outdated
@@ -36,9 +36,9 @@ const exampleTOML = ` | |||
## or in a dependency. | |||
# ignored = ["github.com/user/project/badpkg"] | |||
|
|||
## Dependencies define constraints on dependent projects. They are respected by | |||
## Constraint define constraints on dependent projects. They are respected by |
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 stutters a bit now, with the repetition of constraint
. Let's modify the first sentence to read:
Constraints are rules for how directly imported projects may be incorporated into the depgraph.
manifest.go
Outdated
Ignored []string `toml:"ignored,omitempty"` | ||
Required []string `toml:"required,omitempty"` | ||
Constraints []rawProject `toml:"constraint,omitempty"` | ||
Overrides []rawProject `toml:"overrides,omitempty"` |
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.
As long as we're in here changing things, let's also change [[overrides]]
to the singular, [[override]]
(but keep the property name plural, rawManifest.Overrides
).
c87e45d
to
065c23f
Compare
@sdboyer done! |
OK, this is looking good - thank you! I'm going to wait on merging until we get the other breaking changes sorted, then do them as a batch. |
## Overrides have the same structure as [[dependencies]], but supercede all | ||
## [[dependencies]] declarations from all projects. Only the current project's | ||
## [[overrides]] are applied. | ||
## Override have the same structure as [[constraint]], but supercede all |
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.
"Override" should probably be "Overrides" here to be grammatically correct and consistent with line 15.
Or, this sentence could be "Override has ..." and line 15 can be changed to "Constraint is a rule ..."
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.
Agreed, the former is preferred, primarily for consistency.
## | ||
## Overrides are a sledgehammer. Use them only as a last resort. | ||
# [[overrides]] | ||
## Override is a sledgehammer. Use them only as a last resort. |
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.
"Override is a sledgehammer. Use it only as a last resort."
or
"Overrides are a sledgehammer. Use them only as a last resort."
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.
The latter, please 😄
Gotta chase the conflicts :/ FYI, I'm planning on merging this into a feature branch. Hopefully a short-lived one - I'm aiming for only until Monday. |
Hello, what's the status of this? I can fix the conflict and willing to take it forward on getting merge. |
Bleh, yeah, the conflict will need fixing. The status is that this is waiting to be merged simultaneously with a number of other issues that will be the last backwards-compatibility break with At present, all that's left is #225 and a small addition to #584 that incorporates #421 - that, and, I need to write up some release notes + caveat emptor. |
FYI, this has been incorporated into the |
…ndencies) [[dependencies]] was changed to [[constraint]] but the FAQ had answers that used the old field. Signed-off-by: Ibrahim AshShohail <ibra.sho@gmail.com>
Update the FAQ to use constraint instead of dependencies (reflect #538)
related issue #509