Comment block looks like it's missing a space for alignment

whitespace issues.

whitespace issues

whitespace issues

Same thing about comment block missing an alignment space

indentation

you don't need the #gchar here.

I would say something like "runs @query against @adapter">

Perhaps "A list of message that match @query." reads better.

Missing @query

"Tells @adapter to remove messages that match @query."

"If removing the messages was successful". I wonder if we should make this return an int of how many messages …

These could probably just read %s does not implement the ... function.

There should always be a newline after variable declarations.

wrong indentation

missing active_changed.

This should be something more like "Removes messages from the active #PurpleHistoryAdapter of @manager that match @query."

"removed from the active adapter of @manager."

The #PurpleConversation instance. is plenty.

"Writes @message to the active adapter of @manager."

"%TRUE if @message was successfully written, %FALSE otherwise."

these should be in the same order they are in the Class structure so SIG_ACTIVE_CHANGED should be first.

Not sure how we missed a property for active, but we should probably add it.

This should be moved to line 270 so we don't need to unreference it if we're returning early because it's …

{ should be on a new line.

No need for the cannot perform query, bits in these. Who ever displays the error is likely going to put …

No need for the cannot perform remove, bits in these. Who ever displays the error is likely going to put …

No need for the cannot write message, bits in these. Who ever displays the error is likely going to put …

SQLite is the official capitalization.

don't need the extra blank lines here.

{ should be on a new line as the function parameters were wrapped.

`/ be careful with TRUE as it'll delete everything if theres nothing else in the query. /

we need to g_list_free_full(ins, g_free); each of the 3 lists here.

Might be nice to point out that sqlite prepared statements counts it's parameters starting at 1.

wrong indentation

wrong indentation

{ should be on a new line.

{ should be on a new line.

The check from purple_sqlite_history_adapter_write about checking if priv->db == NULL should be in here too.

we need to finish these.

{ should be on a new line.

The check from purple_sqlite_history_adapter_write about checking if priv->db == NULL should be in here too.

TODO

{ should be on a new line.

return should have an empty line above it unless it's the only statement in a block.

unnecessary newline

{ should be on the previous line as it didn't wrap.

no need for this to be a variable, just put the string in the function call.

need to add purplesqlitehistoryadapter.c as well.

Should just use the same %s does not implement the ... function like purple_history_adapter_query does.

Should just use the same %s does not implement the ... function like purple_history_adapter_query does.

Should just use the same %s does not implement the ... function like purple_history_adapter_query does.

Should just use the same %s does not implement the ... function like purple_history_adapter_query does.

This list shouldn't be sorted.

This should be transfer full and it's returning a list of PurpleMessage's not PurpleHistoryAdapter's.

newly activated

s/from/for/ ?

#PurpleHistoryAdapter

PurpleHistoryAdapter

should be } else {

the { shouldn't be indented

{ should be on a new line when parameters wrap.

{ should be on a new line when parameters wrap.

{ should be on a new line when parameters wrap.

{ should be on a newline when parameters wrap.

use purple_strequal insteqad of strcmp and all four of these.

return's should be separated by a newline unless they are the only statement in a block.

Looks like mixed indentation here.

This should be indented to the level of what it is documenting.

Extra space after (?

Clear at end of test.

Clear at end of test.

Clear at end of test.

Clear at end of test.

replace the query variable with "query" there's no need for this to be a variable.

Clear at end of test.

Clear all these at end of test.

too many newlines

you should be checking the return vaolue of purple_history_manager_remove as well.

you should be checking the return vaolue of purple_history_manager_write as well.

Fix inconsistent names, m->manager, e->error, etc.

Ditto.

This should be query, not test-adapter, I think.

Ditto.

Aren't these all the defaults anyway?

Ditto.

This should go up near GLib and such, not after all the prpl-specific checks.

This should make the command itself exit with EXIT_FAILURE.

message needs to be unref'd before the link is deleted.

"failed to remove"

Should also exit with failure.

I think you can add g_option_context_set_help_enabled on the option context.

This check is redundant with the for loop condition, since you don't do anything different if argc <= 1. You …

THe

should be wrapped.

should be #PurpleHistoryAdapter's

should be #PurpleHistoryAdapter's as #PurpleHistoryAdapters isn't a known symbol to gtk-doc.

Should be #PurpleMessage's

the signals name is registered not adapter-registered so this should be PurpleHistoryManager::registered:

the signals name is unregistered not adapter-unregistered so this should be PurpleHistoryManager::unregistered:

should be wrapped

parameters wrapped so { should be on a newline.

no need to mention what it subclasses PurpleHistoryAdapter

"the instance read and writes to" is redundant and should be removed.

"Creates a new #PurpleSqliteHistoryAdapter."

"(transfer full): The new #PurpleSqliteHistoryAdapter instance"

The parameter documentation shouldn't contain the description of the function. It just needs to include the information about the parameter. …

You need a description here that is separated by a newlines between the parameters and the Returns. Something like *@adapter: …

extra space after *

we should be checking if these are empty strings before adding them to the lists and incrementing the query_items counter. …

need to free the lists here too.

This should be wrapped... something like. g_set_error(error, PURPLE_HISTORY_ADAPTER_DOMAIN, 0, _("Error opening database in PurpleSqliteHistoryAdapter " "for file %s"), priv->filename);

We typically create another variable of the type so we don't need to typecast in the _get_instance_private call which allows …

We typically create another variable of the type so we don't need to typecast in the _get_instance_private call which allows …

with the previous comment, having a variable of type PurpleSqliteHistoryAdapter avoid this line going past the boundary.

We typically create another variable of the type so we don't need to typecast in the _get_instance_private call which allows …

again, use the variable that the previous comment will create to avoid having a line that goes passed the boundary.

We typically create another variable of the type so we don't need to typecast in the _get_instance_private call which allows …

these all should be line wrapped.

putting sqlite3_errmsg(priv->db)); on a new line will get this line under 80 chars.

You don't need the PURPLE_HISTORY_ADAPTER() here.

extra newline

Since TestPurpleHistoryAdapter->query returns a list with 1 item you need to free it after the check with g_list_free.

uncomment and fix typo.

looks like this doesn't need to be wrapped?

looks like this doesn't need to be wrapped?

should be wrapped when over 80 chars.

should be wrapped after 80 chars

you can drop the PURPLE_MESSAGE()

You can drop the PURPLE_CONVERSATION()

This should wrap at 80 chars

previous and current are PurpleHistoryAdapter's not PurpleHistoryManager's.

There should be a space between sqlite3_stmt and the *

should be gint

there should be a new line before return when it's not the only statement in a block.

these three g_list_free_full's can be deleted now as the lists are cleaned up in the loops above and we've transfered …

there should not be a space between * and prepared_statement.

Keep math last.

Missing the PURPLE_IS_HISTORY_ADAPTER check.

Missing the PURPlE_IS_HISTORY_ADAPTER check.

Returns:

Returns:

Extra space.

The old one is deactivated at this point; should it be re-activated?

Should be g_set_error_literal

Should be g_set_error_literal.

Should this be checking conversation or message, or is that dependent on specific adapters?

Should be g_set_error_literal.

SQLite belongs in the title?

SQLite

is a class

defines inherits?

Does something actually derive from this? It should be final if not.

error is already set by g_resource_lookup_data, and error_msg is empty?

purple_strequal returns TRUE when they're equal; this should not be comparing == 0.

This is just split[i][3].

split[i][5]

split[i][0]

Since these are all ORd together, it doesn't appear that there's any benefit to putting this in any 'correct' order, …

Should be g_error_set_literal

If query_items is 0, then none of these would have anything in them.

It would be shorter to write this part of the query as converation_id IN (?, ?, ?, ...).

Same here with author IN (?)

Should be g_set_error_literal.

priv->filename == NULL, so no point in including it in the error message like this, instead of explicitly calling that …

If running migrations fails, then don't you need to close the db handle?

Should be g_set_error_literal.

Instead of SELECT *, you probably should have selected these specific columns. Then you don't need to pick random indices …

Should be g_set_error_literal.

Should you not finalize the prepared statement on error?

This is const.

Should be g_set_error_literal.

Why is this not using the existing message_id?

Should you not finalize the prepared statement on error?

This needs to clear the db handle (and maybe warn) if it wasn't correctly unregistered.

You should pass an error to all these tests and g_assert_no_error.

Query is transfer container, not transfer full. Also, the test adapter returns GINT_TO_POINTER(1), so should definitely not be freed.

Extra line break.

Should be TRUE now.

Should be TRUE now.

You aren't checking the return value to exit with the failure return code.

Clear error.

This indent seems off.

It seems that adding these annotations for the error is unnecessary since the .gir doesn't not record it directly as …

Is this opening in the current directory? It should go to the user config dir.

Can drop the cast.

Should verify that old is not NULL/non-adapter.

Why are there two step calls?

Here are the other ones.