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

Don't allow empty string as description - take II #1831

Merged
merged 2 commits into from
Jul 25, 2016

Conversation

inosik
Copy link
Contributor

@inosik inosik commented Jul 25, 2016

Seems like I forgot this in #1728.

However, I also changed the error message to show the merged metadata, not only the infos from paket.template. I think that was the intent anyways.

With this change, the error message will show the merged (template file
and assembly infos) metadata, not only the information from the template
file.
@forki forki merged commit 0080050 into fsprojects:master Jul 25, 2016
@inosik inosik deleted the bugfix/empty-description branch July 25, 2016 14:01
@richard-green
Copy link

FYI, this has broken our builds. Probably intentionally, but still!

@forki
Copy link
Member

forki commented Jul 25, 2016

I can revert if that makes sense

@richard-green
Copy link

Hi - for now I've pinned our build to use 3.9.1, was more just an informational / warning note to others :-)

@forki
Copy link
Member

forki commented Jul 25, 2016

I reverted it. and releases 3.9.2 is in progress.

@inosik I think we should raise a warning instead.

@inosik
Copy link
Contributor Author

inosik commented Jul 25, 2016

Sorry for the inconvenience, @richard-green, but I think this change is needed. Packages with any of id, version, authors or description, missing, are practically unusable outside of Paket. That's every tool which uses NuGet.Core, that being NuGet command line, NuGet server, Klondike, Squirrel, Octopus Deploy and Visual Studio. Probably some more.

@biehlermi and I had a hard time figuring out why NuGet based test adapters didn't work in a project of ours. The reason was a package created with Paket, which broke VSs discovery process.

See also themotleyfool/Klondike#158.

@inosik
Copy link
Contributor Author

inosik commented Jul 25, 2016

@forki, I would rather let Paket fail, because a warning message is easily overseen in the output of a build script. It also clearly indicates the cause of the failure, instead of an error later in the process, like in themotleyfool/Klondike#158. But I also agree that this is kind of breaking.

@forki
Copy link
Member

forki commented Jul 25, 2016

We could still give it a default description generated from the id and
raise a warning. This would be a soft fix and just silently repair stuff

On Jul 25, 2016 8:04 PM, "Ilja Nosik" [email protected] wrote:

@forki https://github.com/forki, I would rather let Paket fail, because
a warning message is easily overseen in the output of a build script. It
also clearly indicates the cause of the failure, instead of an error later
in the process, like in themotleyfool/Klondike#158
https://github.com/themotleyfool/Klondike/issues/158. But I also agree
that this is kind of breaking.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#1831 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AADgNN8roYEQgECqq56D-VOVvmmz-de3ks5qZPqUgaJpZM4JUHLP
.

@inosik
Copy link
Contributor Author

inosik commented Jul 26, 2016

#1833

@BADF00D
Copy link

BADF00D commented Jul 26, 2016

As @inosik mentioned in https://github.com/themotleyfool/Klondike/issues/158, description is an obligatory field. I'm not a friend of soft fixes. They are almost always a pitfall for anyone who does not know about this.
By the way. We use a paket on a build server. As far as there is no error, nobody ever cares about log files from builds.

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.

None yet

4 participants