Make attribute creation more uniform

[dralley]

closes #410

[dralley]

See question: #410 (comment)

[Mingun]

I think, this looks better:

Suggested change

	/// let features = Attribute::new("features".as_bytes(), "Bells & whistles".as_bytes());
	/// let features = Attribute::new(b"features", b"Bells & whistles");

[dralley]

The type of this is [u8; <len>] rather than &[u8], so you have to do a bit extra (e.g. as_ref(), and at that point it's not really cleaner anymore.

[Mingun]

This is only when you apply that suggestion to the tuple, because (b"...", b"...") will have a type ([u8; <len1>], [u8; <len2>]). Using byte literals with from_* methods is compiled, I checked

[dralley]

I'm not sure how to draw more attention to this but it is a somewhat consequential change and users won't get a compile error.

[dralley]

@Mingun I think it makes sense, now, to make the members of Attribute private and use accessors e.g. raw_value(), normalized_value(), unescaped_value(). Do you agree?

Also, do you forsee any reason for the key parameter type to be Into<QName> as opposed to &[u8]?

[codecov-commenter]

Codecov Report

Merging #413 (6b34514) into master (a6588c2) will decrease coverage by 0.02%.
The diff coverage is 53.84%.

@@            Coverage Diff             @@
##           master     #413      +/-   ##
==========================================
- Coverage   49.58%   49.55%   -0.03%     
==========================================
  Files          22       22              
  Lines       13935    13953      +18     
==========================================
+ Hits         6909     6915       +6     
- Misses       7026     7038      +12     

Flag	Coverage Δ
unittests	49.55% <53.84%> (-0.03%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
src/events/attributes.rs	92.85% <53.84%> (-0.74%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a6588c2...6b34514. Read the comment docs.

[Mingun]

With private fields it will be impossible to use Attribute in patterns which could break some usages. From the other hand, such usage is potentially unsafe, because Attribute could contain data in unknown encoding. I think it is better to introduce such change together with fixing accessors in #379.

Also, do you forsee any reason for the key parameter type to be Into<QName> as opposed to &[u8]?

Oh, yees... I totally forget, that I introduced QName for type safety which means, that user should always explicitly use it instead of plain &[u8], otherwise it just have no point. So yes, it should be QName and could be Into<QName>

[dralley]

user should always explicitly use it instead of plain &[u8]

Well if this is the goal, Into<QName> doesn't make sense and neither does the current From<(.., ..)> syntax. Both of those would allow implicit conversion from bytes (or str).

[dralley]

This PR isn't relevant anymore.