georust / gpx

Rust read/write support for GPS Exchange Format (GPX)
https://crates.io/crates/gpx
MIT License
102 stars 46 forks source link

Allow empty strings in `<text>` and `<type>` of `<link>` #93

Closed w-flo closed 1 year ago

w-flo commented 1 year ago

I've sneaked in a second commit in this PR, to add a changelog entry for my previous PR that was merged without a changelog entry. After looking at the changelog, it seems like my previous decision to skip adding an entry was incorrect (the change probably was more important for users than CI rust version bumps).

Open question for this change: Should <text></text> result in text = Some("") or text = None? I like the current Some("") approach since I suspect some applications use this crate to read, change and then write GPX files. If Some("") was changed to None through some magic, the resulting GPX file would have the <text></text> removed. There would also be some (arguably mostly irrelevant) loss of information ("is there an empty text tag or no text tag?") when reading a GPX file. However, changing it to None might be helpful for applications since most would probably want to treat an emptry string the same as no string. I'd be fine with both approaches.

w-flo commented 1 year ago

Thanks for the review @lnicola :-)

lnicola commented 1 year ago

bors r+

bors[bot] commented 1 year ago

Build failed:

w-flo commented 1 year ago

Thanks for the MSRV bump! I rebased this PR just now

lnicola commented 1 year ago

Thanks!

bors r+

bors[bot] commented 1 year ago

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here. For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.