<p>In app/views/changeset/_changeset.xml.builder:</p>
<pre style='color:#555'>> @@ -0,0 +1,26 @@
> +changeset_attributes = { :id => changeset.id, :created_at => changeset.created_at.xmlschema, :closed_at => changeset.closed_at, :open => changeset.is_open? }
> +changeset_attributes[:uid] = changeset.user.id if changeset.user.data_public?
> +changeset_attributes[:user] = changeset.user.display_name if changeset.user.data_public?
> +changeset.bbox.to_unscaled.add_bounds_to(changeset_attributes, '_') if changeset.bbox.complete?
> +
> +xml.changeset(changeset_attributes) do |asterx|
> +  changeset.tags.each do |k,v|
> +    xml.tag :k => k, :v => v
> +  end
> +  if @comments
> +    xml.discussion do 
</pre>
<p>First impressions:</p>

<p>Having a builder for the returned changeset XML from <code>:read</code> is a departure from the way we've done things in the past, and means we now have changeset XML defined in two places. This isn't a big deal, but might lead to inconsistencies between the output of <code>:read</code> and anywhere <code>Changeset.to_xml</code> is used.</p>

<p>The <code>:text</code> parameter of <code>:comment</code> is from the url-encoded POST body, whereas typically we would take API input as an XML document. I don't think this is a problem, but could lead to inclusion of characters <a href="http://www.w3.org/TR/REC-xml/#charsets">not representable in XML</a> which will need to be scanned for and a 400 Bad Request returned if any are present. I also note there's no length restriction on the text, which might be worth having.</p>

<p>I didn't notice (perhaps I missed it) what the document type of the text is? Is it HTML or Markdown or just plain text?</p>

<p>What's the rationale for delaying commenting on or subscribing to a changeset until after it has closed? I can see that it complicates matters (as in this github thread) when discussion begins and comments are then "outdated" by subsequent changes, but it also has the side-effect of potentially making users wait an hour for changesets to close from time-out if the editor used doesn't close them.</p>

<p style="font-size:small;-webkit-text-size-adjust:none;color:#666;">—<br>Reply to this email directly or <a href="https://github.com/openstreetmap/openstreetmap-website/pull/772/files#r15986417">view it on GitHub</a>.<img alt="" height="1" src="https://github.com/notifications/beacon/1419053__eyJzY29wZSI6Ik5ld3NpZXM6QmVhY29uIiwiZXhwaXJlcyI6MTcyMzExMjg4MiwiZGF0YSI6eyJpZCI6MzU4NjU3NjJ9fQ==--a783bd0bb77d8c0fcdba59c2ec28dc80846de274.gif" width="1" /></p>