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

Updated version constraint copy #5165

Merged
merged 2 commits into from
Sep 19, 2023
Merged

Conversation

atsansone
Copy link
Contributor

@atsansone atsansone commented Sep 12, 2023

@atsansone atsansone added the review.tech Awaiting Technical Review label Sep 12, 2023
@MaryaBelanger
Copy link
Contributor

@atsansone can you stage this?

[`--enforce-lockfile`]: /tools/pub/cmd/pub-get#--enforce-lockfile
[lockfile]: /tools/pub/glossary#lockfile
[content hashes]: /tools/pub/glossary#content-hashes
[`--enforce-lockfile`]: {{site.url}}/tools/pub/cmd/pub-get#--enforce-lockfile
Copy link
Member

Choose a reason for hiding this comment

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

I haven't taken a look at this PR yet, but just scanning quickly, we don't use {{site.url}} on dart.dev. Just the base relative URLs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

True, and I will remove. I think we should going forward to be consistent between both sites.

Copy link
Member

Choose a reason for hiding this comment

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

That's the goal. I hope to make it no longer necessary on flutter/website soon.

@parlough
Copy link
Member

Consider getting someone on the pub team to review this as well once it is staged.

@parlough parlough added the review.await-update Awaiting Updates after Edits label Sep 13, 2023
@atsansone atsansone requested a review from jonasfj September 15, 2023 18:06
@atsansone atsansone assigned jonasfj and unassigned atsansone Sep 15, 2023
@atsansone atsansone requested a review from parlough September 15, 2023 20:05
@atsansone atsansone removed the review.await-update Awaiting Updates after Edits label Sep 15, 2023
src/tools/pub/dependencies.md Show resolved Hide resolved
Here's an example of specifying a dependency:
[_source_]: {{site.url}}/tools/pub/glossary#source

To specify a dependency:
Copy link
Member

Choose a reason for hiding this comment

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

The old text does make clear that this is an example.

To me it wrong to claim that this how an example

Comment on lines 239 to 240
How can you communicate to other developers which version of Package B
has been verified with a given version of Package A?
Copy link
Member

Choose a reason for hiding this comment

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

verified is a strong claim here.

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe use 'compatible'?

Copy link
Contributor

@szakarias szakarias left a comment

Choose a reason for hiding this comment

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

LGTM with the comments

<em>exact</em> version. Avoid using this when you can because it can cause
version lock for your users and make it hard for them to use your package
along with other packages that also depend on it.
You can specify any combination of version values as their ranges intersect.
Copy link
Contributor

Choose a reason for hiding this comment

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

What it is exactly we mean here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have a better explanation in the updated copy.

Comment on lines 239 to 240
How can you communicate to other developers which version of Package B
has been verified with a given version of Package A?
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe use 'compatible'?

Copy link
Contributor Author

@atsansone atsansone left a comment

Choose a reason for hiding this comment

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

@jonasfj , @szakarias , @parlough : Made updates per your comments.

src/tools/pub/dependencies.md Show resolved Hide resolved
<em>exact</em> version. Avoid using this when you can because it can cause
version lock for your users and make it hard for them to use your package
along with other packages that also depend on it.
You can specify any combination of version values as their ranges intersect.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have a better explanation in the updated copy.

[`--enforce-lockfile`]: /tools/pub/cmd/pub-get#--enforce-lockfile
[lockfile]: /tools/pub/glossary#lockfile
[content hashes]: /tools/pub/glossary#content-hashes
[`--enforce-lockfile`]: {{site.url}}/tools/pub/cmd/pub-get#--enforce-lockfile
Copy link
Contributor Author

Choose a reason for hiding this comment

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

True, and I will remove. I think we should going forward to be consistent between both sites.

@parlough parlough assigned parlough and unassigned jonasfj and MaryaBelanger Sep 18, 2023
@parlough parlough added review.copy Awaiting Copy Review and removed review.tech Awaiting Technical Review labels Sep 18, 2023
@atsansone atsansone merged commit 314b9ea into dart-lang:main Sep 19, 2023
9 checks passed
atsansone added a commit to atsansone/site-www that referenced this pull request Jan 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
review.copy Awaiting Copy Review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Clarify caret syntax
5 participants