Closed spences10 closed 7 years ago
Nice work @spences10. Looks like a good start to the XL<->XML feature set.
I'll try to test it soon. Have you tested this out and got it working yourself? I am skeptical about the use of "ThisWorkbook" instead of "ActiveWorkbook", but maybe I'm just confused.
I have a lot of thoughts about refinements, and will submit them as a review soon. Would you like to merge as soon as possible and refine it later or deal with refinements before merge?
HoLeeShit @mattpalermo!!
Ok, they're all valid points, some of them I find a bit hard to understand what you want changing due to the GitHub interface
It was a first attempt just to get something up there working, I've done a bit more work on it
Sorry haha. I got a bit carried away. As I mentioned though, there are only two really critical issues, the rest are simply things that I think would improve the quality of the code.
Hey, thanks just submitted 916a66f30fe7319ec0c08ff1f8c8de869f1d88e6
It is good work though :+1: you've figured out the hard bits.
@mattpalermo tried to answer most of your points raised mate.
Just to recap where we are with this now:
ActiveWorkbook
schoolboy error on my part!What I haven't covered:
^^ is this just hardcode in the config??
Yea, I am mostly just concerned with the CodeExport.config.json
file. It can be either hand crafted or generated with the Make Config File
button. Checkout my recent README file changes. That might shed some light on the purpose of keeping the CodeExport.config.json
file updated.
@spences10 facepalm haha. I love those badges! And I'm starting the think you might love badges as well 🤔
Yes my friend!!
Get those AGD all over the place!!
Resolves #57
@mattpalermo review and merge pls sir :tophat: