Skip to content

Break deprecated tasks DownloadPackageV0, NuGetInstallerV0, NuGetRestoreV1 #19325

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

Merged
merged 22 commits into from
Dec 13, 2023

Conversation

amp-powell
Copy link
Contributor

@amp-powell amp-powell commented Nov 28, 2023

Task name: DownloadPackageV0, NuGetInstallerV0, NuGetRestoreV1

Description: Breaking tests after they complete to make it clear that they are deprecated

Documentation changes required: (Y/N) Y

Added unit tests: (Y/N) Y

Attached related issue: (Y/N) Y - User stories 1662927 and 2075334

Checklist:

  • Task version was bumped - please check instruction how to do it
  • Checked that applied changes work as expected

@amp-powell amp-powell requested a review from a team as a code owner November 28, 2023 00:14
@amp-powell amp-powell changed the title Users/abpowell/break deprecated tasks Break deprecated tasks DownloadPackageV0, NuGetInstallerV0, NuGetRestoreV1 Nov 28, 2023
Copy link
Contributor

@magleaso magleaso left a comment

Choose a reason for hiding this comment

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

Error messages should be more direct, should use loc strings whenever possible, and consider whether throwing at the end of the execution of the task is the best way

@geekzter geekzter mentioned this pull request Nov 30, 2023
2 tasks
@amp-powell amp-powell requested a review from magleaso December 12, 2023 01:54
@amp-powell amp-powell dismissed magleaso’s stale review December 12, 2023 01:56

Made strings clearer, using loc strings, and had a meeting about breaking at the end vs. beginning

@amp-powell amp-powell merged commit afd241c into master Dec 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants