Closed sjbreiner closed 1 year ago
Hi @olynch: I cleaned up the junk files and comments in this PR
Added Tikz export
Hi @olynch:
Do you want to review this giant (sorry) PR?
I'd like to get the main repo caught up with my changes.
There are still some review comments I've already given which are unaddressed, in addition to failing formatting and build checks.
There are still some review comments I've already given which are unaddressed,
What are you looking at? As of commit 8581002 I think I got the stray comments and println's.
in addition to failing formatting and build checks.
I don't know how to fix this, but would really like to.
Oh, now they are showing as fixed! I think my tab was just old :P
I don't know how to fix this, but would really like to.
Run mill core.reformat
and mill apps.<each app>.reformat
For the upload check, you should make sure that mill apps.<each app>.fullLinkJS
passes for every app that's enabled in the build.sc
.
It looks like the upload is failing because of AWS credentials issues; I'll try to fix that.
Looks like that fixed one of the issues but not the other
Hi @olynch,
I've addressed most of your requests, with some additional comments for the ones that I didn't fix directly.
Let me know if you have anything else.
Thanks, Spencer
Is this waiting on the failed build?
@sjbreiner there are a lot of comments of mine that you marked resolved, but I'm not seeing that you changed anything... maybe you forgot to push something?
Hi @olynch,
Probably my confusion about how git works.
As of the last "reformat" commit on my repo listed above (032322e), those issues are fixed. E.g., for the missing comment on dragMove
, my repo now looks like this:
However, I don't see those changes in the github pull request dialogue:
I thought the PR would pull in those later changes automatically, but maybe I need to do something to update the PR?
Thanks, Spencer
@sjbreiner I see you've marked some issues as resolved, but haven't pushed new commits yet?
Hi @olynch,
I created an issue for pulling in the JSON file, and I think I addressed all the other issues.
Any progress on the AWS issue? I'm looking forward to closing this one out (and adopting better development practices in the future :rofl:)
Hi @olynch,
Any progress on this build issue?
I'm also curious about whether we can add the reformat
steps into the checkFormat
script, rather than doing it manually before commit?
Merged in #116
This update includes: