Conversation
githubdoe
left a comment
githubdoe
left a comment
There was a problem hiding this comment.
I don't understand what changes were made to astigPolargraph. The diff on the left has my code highlighted in red. With that code missing on the right view. To me that means my code was deleted. Which is not what I think will work.
|
I tried the build from this PR downloaded here https://github.com/githubdoe/DFTFringe/actions/runs/15051150032?pr=186 and I confirm it doesn't fail on my machine. I'm not saying you didn't had a problem. But the source is probably not this code. It might fail later somewhere else... |
|
Sorry I can not make any sense of what is the final code. But I will not approve any change of moving the routines together since it does not work on my machine. It has to work on my machine. |
|
Have you downloaded the build from the link ? Might be from your build and not from the code itself. If so what are the steps to reproduce the problem ? And can you share the log file ? Crash should be logged. |
|
I don't know how to get your code. I pulled master but that does not have your changes. I need to use my build and then I can try your version of the code. |
|
It works fine for me Dale. I went here: And at the bottom of the page it has black bold text which is actually a link and downloads the installer. I installed with the installer and ran it and it was fine. If you want to compile it yourself it is trickier - you can click on the "code" button on the top right of this page and it suggests to use this command: But you need to install the github cli for that to work. |
|
Julien, you did exactly what dales code comment said not to do. He says it used to crash that way and so he created the function makeChart(). But then you undid his (weird) fix. Can't we leave it the way Dale had it? |
|
Ok ok. Let's be clear. I agree I did exactly what dale told not to do because not working. However this makes sense to none of us. And 100% of the time I have been in such a situation the outcome is "this wasn't actually the issue" or "the fix wasn't actually a fix". This will definitely backfire OR is fine the way I propose. I have no problem delaying the merge. However I want to dig this to the end and understand what's wrong exactly. If something is actually wrong. Dale it would really help me if you could download the installer like explained by George and see if it fails or not. |
|
Yes just opening the window. If you could point me to your file that I can copy to my build I will try your changes. But I want to make sure they are your changes and not something I tried to mimic. It must be in Git somewhere I just don't know what branch to look at. |
|
Thanks george. As I have no rights in this repo it's on a branch on a different remote. Let's not try to find it, would be quite complex. here is the file alone if you prefer the download of whole repo as is in this PR : https://github.com/atsju/DFTFringe/archive/refs/heads/JST/minorFixes.zip Most logical outcome would be your local build to fail (as per your comment in code). But I hope the github installer build works OK. Let's try both. I agree all "need to work on your machine" no wories |
|
The problem is that it's in Julien's repository. You could for example switch from githubdoe to atsju (github.com/atsju/) then locate the DFTFringe repository there and then find the branch "JST/minorFixes". Or here is a direct link: https://github.com/atsju/DFTFringe/tree/JST/minorFixes Then in the upper right corner you could clone that repository (but don't get it confused with your repository which already has several branches in it so you would put that in a newly created folder). Then you could switch to the JST/minorFixes branch. Or, much easier and probably safer, install the github CLI. I have it but I've only used it like 3 times ever. It certainly makes life extremely simple if you want to build a branch like this. Or Julien (or I) could probably merge these changes into the dalework branch on the githubdoe account and you could just git pull the changes. But then if you don't want them we would have to revert the changes in your dalework branch. |
|
Or we could tell Julien we aren't pulling this PR. And he should fix the logging thing in a new PR. |
I would be fine with this too. (Tomorrow) |
|
It's possible the crash only occurs when in DEBUG mode. Typically in DEBUG mode all arrays and things are a bit larger than default. Also some crashes appear to be in a particular line of code but actually occur somewhere totally different but they mess up the stack so the debugger is all confused. If it's a repeatable crash you can put a break point just before the crash and step one line at a time to be sure the crash is happening where you thing it is. |
Yeah it's probably 10pm (22:00) where Julien is. |
|
No it crashed in both release and debug. I will try the .txt file. I have several things going on that need my attention today. |
|
Ok I tired the code which is exactly like my first try and it no longer crashes. Even went back to dalework and it works there now as well. Good. I will approve. |
|
Once more Git has mystified me. I can't find a link where I have to approve. |
|
Top right corner of this page. click green button. May say "add your review" or "review code" or similar. Check the approved radio button. |
|
There was no green button at the top right corner which is what I was expecting in the first place. I eventually clicked around on things up there and got it to get me to the code review screen. I approved it I think. |
|
Nice. Thank you both ! |
3 participants
No description provided.