-
Notifications
You must be signed in to change notification settings - Fork 145
Code Snippets are not displayed in gist overview #110
Comments
Thanks a lot @K-lemens for reporting this. |
@jack-chapman do you think that this bug could be caused because of 8b53008 ? |
@kodaman2 @flashadvocate, may I have your help too please? |
I can confirm this bug as of the #95 PR. I did notice that switching contexts sometimes made it work. It affected some snippets but not others. |
Will definitely need some good debugging can't make any promises this week.
Maybe until the weekend I'll take a look.
Probably a good idea to add some integration unit testing to catch this UI
rendering issues too.
…On Mon, Nov 4, 2019, 9:56 AM Chris Deaton ***@***.***> wrote:
I can confirm this bug as of the #95
<#95> PR. I did notice that
switching contexts sometimes made it work. It affected some snippets but
not others.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#110?email_source=notifications&email_token=ACOKIUVRKNHG3FS7QT2OLTDQSBAZJA5CNFSM4JIUGBKKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEC7XPTI#issuecomment-549418957>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ACOKIUQHOO2GUYHJDMJHQIDQSBAZJANCNFSM4JIUGBKA>
.
|
No problem @kodaman2. |
Wanted to let you know that in v1.2.2 it worked without a flaw. |
I got a quick test done PR #95 is not the problem, here what I did to test it. Pull code from commit 5991e90 which was before PR #74, then manually added the code changes from PR #95, I then proceeded to:
Works perfectly fine. pinging @marvpaul for his input. I still don't understand how I can't reproduce the bug with @lauthieb I think for the future it would be good to run builds for each PR that's modifying core features to make sure nothing breaks. Not saying PR #74 is at fault since there were lots of commits added to master since so is going to be tough finding what actually broke the notes displaying. |
@kodaman2 thanks for pinging me. I started a debug session for the production app and figured out that this error seems to be the problem What else I have done:
So hopefully I can say the reason for that bug lays in a commit somewhere between 29d3a74 and 1340843 My guess is that this commit causes the problem: 8b53008 I'm out for now. Perhaps someone else will have some time to further investigate the reason for this problem and hopefully can confirm what I've found out so far. |
That is a very good assessment @marvpaul , if this was my repo, I would revert back to that commit which broke the app, then manually add the following code commits after that without any major changes once is all tested and built then I would work on major vue changes and stuff like that. I think the process was a bit loose along the way. Just my 2c lol don't take it personal. |
I'm not an expert on electron apps but shouldn't this bug be visible during |
I had some time today to further analyze the source of this bug. @jack-chapman do you have an idea what could be the problem? At first I though it could be a problem with the updated Electron version (perhaps a bug?), but I tried to do a build with electron v. 1.8.8 on commit 8b53008 and the bug still appears. Perhaps a problem regarding mini-css-extract-plugin? Someone has reported a bug which produces a similar error message (--> webpack-contrib/mini-css-extract-plugin#257). I'm not to much into topic, but it could be worth a try to exclude mini-css-extract-plugin I think. |
Good luck finding this bug gents, tons of npm packages were upgraded. I would highly recommend re-doing #93 in very short commits, #93 has only 4 commits with the first one has really all the changes already. So is very difficult to point to which a specific package, code change broke the app. I think the issue is related to the vue templates edits + package upgrades, I tried to step trace back the changes, but I think it would be far easier to just remove it all together from the baseline, and create a dev branch to do this vue upgrade more carefully of course with build tests as the upgrade progresses. @lauthieb if you want to go this route I can do it, or perhaps @jack-chapman has a better idea. |
Hi @kodaman2, thanks a lot for your investigation. Feel free to begin a new branch and try to resolve this bug. I can’t do this for the moment, my work takes me too much time unfortunately :( but I’ll try to answer you ASAP if you have any question on this subject. Many thanks another time. |
No worries mine too, I'll try to work on it here and there on my evenings and weekends. I was just planning for now to subtract the commit 8b53008 from master and submit a PR so master is restored to fully functional again. After that I could try to redo that 8b53008 commit on another branch but with smaller commits to try and find the package causing the issue. Or @jack-chapman can redo it on top of the latest master after I am done since it was his original commit, if he wants to do it, either way works with me.
|
Hi @kodaman2, did you work on this or no? |
@lauthieb
I've tried a few times with no luck, you downgrade one package and the
whole thing crumbles. As much as I hate to say it, I think the easiest
thing to do is to reset master to a working commit, then ping each individual PR
that was merged after that to re-submit their PR or else it becomes a lot of manual labor. Though I'll probably stay away from 8b53008 until master is restored.
Going forward I'd be a bit more careful about merging commits without doing
a production build and dev test. Or better yet add that to a Contributions.md and/or PR template to ensure contributors test their PRs well.
Anyone got any other thoughts on how to fix this? I agree with @marvpaul if you launch the production app with dev tools and follow the console error, the error comes down to the minified code though I tried reverting `.electron-vue` files that were changed to change the css plugin, and downgrade some packages after doing a diff and I am unable to `npm run dev` or even `npm run build`.
…On Mon, Nov 25, 2019, 4:37 PM Laurent Thiebault ***@***.***> wrote:
Hi @kodaman2 <https://github.com/kodaman2>, did you work on this or no?
I've updated our website to reference the 1.2.2 in the download's section
for the moment.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#110?email_source=notifications&email_token=ACOKIUSB5JXEZ5X2CGYZ573QVRHR5A5CNFSM4JIUGBKKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEFEBKVQ#issuecomment-558372182>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ACOKIUUTV5SU5CAVWL2QP2LQVRHR5ANCNFSM4JIUGBKA>
.
|
Thanks to a great PR of @kodaman2, this issue is now fixed. |
After downloading and updating to the newest version of Code Notes (v. 1.2.3) my Snippets stored in Gist are not displayed correctly. The titel of each Note is shown, but the Note is displayed empty. Though when I click on "Edit Notes" the full text, which I also can see in my Gist online, is displayed and can be edited.
Screenshots
Desktop (please complete the following information):
The text was updated successfully, but these errors were encountered: