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

Get rid of obsolete notifications on package details view #4902

Merged
merged 3 commits into from
Oct 25, 2017

Conversation

xavierdecoster
Copy link
Member

Fixes #4788 (see issue for details)

@joelverhagen
Copy link
Member

I see your comment to @anangaur, but it seems weird to leave the "edit failed" message if we are removing the "pending edit". I think we should remove both at the same time, but your call!

@@ -11,7 +11,6 @@ public enum AuditedPackageAction
List,
Unlist,
Edit,
UndoEdit,
Copy link
Member

Choose a reason for hiding this comment

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

Consider marking this as [Obselete] instead of removing it.

Copy link
Member Author

Choose a reason for hiding this comment

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

That's an option too. Is this used elsewhere? And if it is, shouldn't we break upon upgrading the consumed NuGetGallery.Core dependency to bring it under our attention (not sure an Obsolete warning message is intrusive enough :)).

Copy link
Member

Choose a reason for hiding this comment

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

I am more thinking that since it's a serialized enum, it will allow us to deserialize the audit later.

Copy link
Member Author

Choose a reason for hiding this comment

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

Gotcha. Marked as obsolete in 26074f4

@xavierdecoster
Copy link
Member Author

@joelverhagen the thing is that edits may still be pending at the time of deployment, so we may need to remove this particular Edit-Failed notification in a second deployment?

@joelverhagen
Copy link
Member

@joelverhagen the thing is that edits may still be pending at the time of deployment, so we may need to remove this particular Edit-Failed notification in a second deployment?

Seems fine. I think failed edits are a very rare occurrence.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants