Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix flicker and auto run #105

Merged
merged 3 commits into from
Sep 1, 2023
Merged

Fix flicker and auto run #105

merged 3 commits into from
Sep 1, 2023

Conversation

Shaikh-Ubaid
Copy link
Member

@Shaikh-Ubaid Shaikh-Ubaid commented Sep 1, 2023

fixes #101

fixes #99

fixes #103

towards #104

@github-actions
Copy link

github-actions bot commented Sep 1, 2023

Your site is deployed at https://lfortran.github.io/pull_request_preview/lfortran/105

@certik
Copy link
Contributor

certik commented Sep 1, 2023

Not sure what is going on:

Screen Shot 2023-09-01 at 1 56 08 PM

@Shaikh-Ubaid
Copy link
Member Author

Shaikh-Ubaid commented Sep 1, 2023

For every preview we have two build processes. One happens in this repository and pushes to the repository pull_request_preview. After the push, the CI at that repo (which is the default github-pages deploy provided by GitHub) is triggered. The CI at that repo seems to be executing currently which we can track here https://github.com/lfortran/pull_request_preview/actions (the first one from this list).

PS: It probably got completed in 4 minutes. This is similar to the issue we were facing here #94 (comment).

@certik
Copy link
Contributor

certik commented Sep 1, 2023

Ah got it. It works!

Here is what I am getting https://lfortran.github.io/pull_request_preview/lfortran/105?gist=certik/7e2652943bbff7f0d0963dd4fcf1813a

Screen Shot 2023-09-01 at 2 01 45 PM

But some other times it works great. It could be a caching problem, not sure.

@Shaikh-Ubaid Shaikh-Ubaid marked this pull request as draft September 1, 2023 20:03
Also set default file name to main.f90
@Shaikh-Ubaid Shaikh-Ubaid marked this pull request as ready for review September 1, 2023 20:15
@github-actions
Copy link

github-actions bot commented Sep 1, 2023

Your site is deployed at https://lfortran.github.io/pull_request_preview/lfortran/105

@Shaikh-Ubaid
Copy link
Member Author

Shaikh-Ubaid commented Sep 1, 2023

It #105 (comment) is fixed now.

But some other times it works great. It could be a caching problem, not sure.

I had noticed it once or twice while working on the PR, but I couldn't reproduce it. I also assumed it could be a caching issue. But since you also experienced it, I pondered over the code only to realize that it is a common issue experienced with callbacks. This happens specifically in the case of the param gist, where we use a callback function that updates the value of the source. Callback functions are added to the Callback queue and executed later when the call stack is empty. The issue happened because fetchData becomes true (that is we now run the source code, since data is fetched), while the update source code instruction is waiting in the callback queue (it means we have the data but we did not yet update the value of source code with that data). I updated the code to set fetchData to true only/right after the update source code instruction thus fixing the issue.

@github-actions
Copy link

github-actions bot commented Sep 1, 2023

Your site is deployed at https://lfortran.github.io/pull_request_preview/lfortran/105

@Shaikh-Ubaid
Copy link
Member Author

I think this fixes 3 of the issues at #104.

Copy link
Contributor

@certik certik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this works!

Let's merge it, and we'll see how it goes. If I discover any further issues, I'll report them.

Thanks @Shaikh-Ubaid !

@certik certik merged commit 1e8bdb3 into main Sep 1, 2023
1 check passed
@certik certik deleted the fix_flicker_and_auto_run branch September 1, 2023 22:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants