-
Notifications
You must be signed in to change notification settings - Fork 60
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
Reinstall islandora & views for make local #213
Conversation
On step 5 above, I'm still getting the error. Both times I run
The error from logs before triggering the error and running
The error from logs after running
|
No idea if this is related to why it didn't work, but this is what I got with my terminal taking up half my screen vertically:
I.e. the command found nothing because the line is wrapped in the display, which really seems like it should not matter. I make my terminal fullscreen and get:
I wonder what the default terminal/bash line length on where this runs internally is set to or what it is dependent on. Either way, bad news... I changed the first bit of the script to the following to avoid the brittleness of grepping something pre-processed for display and now the script appears to be running:
|
Yes, and once that script did finish running successfully, the error is fixed. |
I think this PR should also include adding a section to docs/troubleshooting.md to explain that you should run |
@kspurgin Thanks for reviewing with suggestions and sorry for not seeing you comments earlier. This should be ready for review. |
After Not sure about the cause of the issue though. Wondering if we need to incorporate this a part of |
I assume it won't hurt anything but it will enable all views including those that were previously disabled. |
I ran through the steps test this PR and it worked as expected. BTW, I looked at the proposed docs changes, and I wanted to suggest that we also mention that this issue may alternatively show up with getting a blank page that just says "The website encountered an unexpected error. Please try again later." Of course, there are other reasons why you may get an error like that, so we should warn about that too. Also, assuming I understood how this PR works, should there be a small note that all views will be enabled in your Drupal instance after running this fix. Just in case folks end up enabling views they rather keep disabled? I can volunteer to write these docs suggestions if they make sense others. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR works as expected.
Two observations
- I added some feedback on a comment on this PR on March 16 on potentially adding documentation related to this code change.
- In that same comment I wondered if there should be a documentation addition or warning when running this makefile command that all views will be set to enabled. If I am understanding the code, this may be important for users to be aware.
Resolved merge conflicts, now ready to test again. |
@ysuarez Good point. Maybe I should add a little logic to fetch a list of which ones are enabled and only trigger those. |
I will test this early next week. |
@ysuarez Any chance you would have time to review this? No pressure, just reviewing open PRs in preparation for the conference. |
Don, my bad. I will try to test this by next week's tech call. Right now I am still trying to finish testing the islandora_defaults PR 66. I will put in some time while I test that other PR to test this one, since yours should be pretty simple to test. |
@DonRichards This PR works as expected. |
This may need some refinement but after a lot of testing I notices disabling then re-enabling the views with a module reinstall fixed the issue. It seems to have fix the issues described in #212
and #136
What changed
Added a make command to fix issue and a custom script that will only fix the issue if it's actually the issue.
To test (
make local
only)Assuming you've ran
make clean
followed bymake local
but not have triggered the error yet. If you have either run the previous steps (to check the skip works) or just skip to step #4.make fix_views
and it will try to copy the file into the container, run it and remove the script.What just happened
Before attempting to make a page there was no error triggered. In the shell script is a check to look for a specific error. This way the fix is only applied when it absolutely will address the issue.
Other issues addressed
Wasn't sure the impact of the missing config/sync directory and the masonry errors. So to eliminate those as compounding factors I addressed them in this PR as well as the "flysystem module is missing" errors. Some part of the build process is expecting it.