Bake in relocatable data

[nhz2]

Fixes #325 by replacing the artifacts with baked in data.

This makes it easier to change the template xml data.

Relocatability tested with https://gist.github.com/nhz2/fef85cf22bef076348e921f5c46be2d3

[codecov-commenter]

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.63%. Comparing base (4bf7cde) to head (bb085e4).
⚠️ Report is 3 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #351      +/-   ##
==========================================
- Coverage   94.63%   94.63%   -0.01%     
==========================================
  Files          21       21              
  Lines        7052     7046       -6     
==========================================
- Hits         6674     6668       -6     
  Misses        378      378              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[TimG1964]

Thank you for doing this.

I can see the value of moving metadata.xml from a const in the code to a data a file. But I'm not sure I understand the benefit of baking in these three "data" files in /src rather than simply putting them in /data. I'm sure the benefits are real, I just don't understand them.

Are the two files that were in /data now duplicated in both /src and /data? Is that necessary?

[nhz2]

Good point, I see there is now a slight difference where one file is using "LF" and the other is using "CRLF" I'm adding a .gitattributes https://code.visualstudio.com/docs/remote/troubleshooting#_resolving-git-line-ending-issues-in-wsl-resulting-in-many-modified-files to hopefully avoid that changing in the future.

The idea is to seperate out the files needed for running XLSX.jl and put those is "src" and the files that are used only for testing, which are in "data", but will eventually go in a testing artifact.

[felipenoris]

This is interesting, given that include_dependency makes this kind of solution relocatable.

The Artifact system was introduced in XLSX in #127 to make it relocatable.

I updated the comments on #325 with instructions on how to update artifacts.

I think, intuitively, that the Artifacts solution is better suited for this, given this is not source code content, but static data. But I understand this move if it makes stuff easier to update (if you think you'll be updating XML often).

[nhz2]

There is also a performance improvement since this PR skips the need to do a file system lookup after pre compilation. Since the files are small I think it is okay for them to sit in memory after using XLSX.

[TimG1964]

Thanks you both very much for your advice and support and @nhz2 for this PR.

I still feel that I don't fully understand all the issues here. This is down to me. I need to do more work, perhaps with a toy project, to make sure I understand things properly.

It seems to me you are both advocating broadly the same thing - use Artifacts for all the files used in package tests - but @nhz2 is also suggesting treating the three files needed by the package itself differently.

This issue seems important but, since it relates to "internals", is probably less urgent than releasing a v0.11.0. I think I will make a new release now and leave this PR open. I hope to come back to it very soon, once I've improved my understanding.

Thanks for your continued support!

Tim

[nhz2]

This PR basically makes the const workaround from #325 (comment) the way to do things. But instead of copying the data from the file into a string literal, in this PR I'm using the read function to automatically do that. This way there is one source of truth.

The include_dependency is used so that changes to the files re-trigger precompilation.

Relocatability is really tricky to reason about so I made a helper script to check if a package is relocatable with a direct test. https://gist.github.com/nhz2/fef85cf22bef076348e921f5c46be2d3