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

Provide an error message as warning output #101

Merged
merged 1 commit into from Jan 19, 2017
Merged

Conversation

aleixpol
Copy link
Collaborator

As discussed in the issue #97, there's some systems where appstream is
reporting that it doesn't load properly but the users on the platform
don't have information on how to fix it.
With this warning, they will at least have a chance to know.

@@ -66,7 +66,13 @@ Pool::~Pool()

bool Pool::load()
{
return as_pool_load (d->m_pool, NULL, NULL);
GError* error = nullptr;
Copy link
Owner

Choose a reason for hiding this comment

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

Ideally use g_autoptr here:
g_autoptr(GError) error = nullptr;
(then you can get rid of the g_error_free line)

@ximion
Copy link
Owner

ximion commented Jan 11, 2017

I always dislike libraries printing stuff to stdout/stderr, so while I would be okay with this change, I think it's just a workaround. Can we propagate the error in a useful way somehow? (by returning an error struct, using C++ errors, or adding a lastError QString property (or struct, if we want to preserve the error code) to the class or doing something else.)

@aleixpol
Copy link
Collaborator Author

I see your point, but it's not like the application can do anything other than showing the error message in a fancy pointless way.
In this case it's an error that needs to be fixed by the distribution and the distribution alone. The user shouldn't be responsible for it.

@ximion
Copy link
Owner

ximion commented Jan 12, 2017

The application using the library might want to show the information in an unobtrusive popup, add more information to it and display it, write it to its own logfile and send it to the developers with other diagnostic info and possibly other things I don't know of.
We can't know how users of libraries will use them, so giving people all important information in an easy, programmatically accessible way is key.

(I think we can still merge this, but I would like a consistent way to report errors in AppStreamQt much more)

As discussed in the issue ximion#97, there's some systems where appstream is
reporting that it doesn't load properly but the users on the platform
don't have information on how to fix it.
Provide a warning for frontends to display, giving indications on what's
the problem.
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

2 participants