Jump to conversation
Unresolved conversations (0)
Nice work!

Nice work!

All of your conversations have been resolved.

Resolved conversations (4)
@ximion ximion Sep 9, 2021
Please use `as_xml_add_text_prop` here, it's a bit nicer for newly written code. So these whole two lines (you can drop the null check) would just become a single line: ```c as_xml_add_text_prop (n, "source_locale", priv->source_locale); ```
Outdated
src/as-translation.c
pwithnall
@ximion ximion Sep 9, 2021
Please use `as_xml_get_prop_value (node, "source_locale")` for new code (just a convenience macro, but it improves readability and makes refactoring easier). You are also leaking memory here, as the result of `xmlGetProp` needs to be freed. You can solve all of this with just a few convenience functions: ```c as_ref_string_assign_transfer (&priv->source_locale, as_xml_get_prop_value_refstr (node, "source_locale")); ```
Outdated
src/as-translation.c
pwithnall
@ximion ximion Sep 8, 2021
We should use snake_case instead of kebap-case for the new property, so `source_locale` please. This is simply because the rest of the specification uses snake_case for properties and tag names, and consistency is important :-)
Outdated
docs/xml/collection-xmldata.xml
pwithnall
@ximion ximion Sep 8, 2021
Typo, makes the CI complain about this PR currently ;-)
Outdated
docs/xml/collection-xmldata.xml
pwithnall