datacarpentry / organization-genomics

Project Organization and Management for Genomics
https://datacarpentry.org/organization-genomics
Other
23 stars 76 forks source link

Change page.root to relative_root_path for correct CSS rendering #76

Closed tstilwel closed 5 years ago

tstilwel commented 5 years ago

PR originating from #75 and similar to #73.

hoytpr commented 5 years ago

Excellent! Do you have a link to your page with the changes made? I could pull them onto mine but it should be easy to just point to yours using your css-fix branch. Thanks!

tstilwel commented 5 years ago

Is this what you're referring to? Sorry, I'm still getting comfortable with Git.

2bd0ddd667fa656a8c85bd500249ed8a3bb0f99f

hoytpr commented 5 years ago

No, I was wrong to ask that. I fully understand that Git isn't easy. I'll have to try to view this myself before we can merge it.

Thanks very much!

hoytpr commented 5 years ago

Hey @tstilwel if you are still there, and have time, could you send that pull request to a test site I set up? the site is: https://github.com/hoytpr/testsite2 Then if it works on the testsite, I can merge it here! Thanks very much for your effort! Pete

hoytpr commented 5 years ago

@tstilwel I got the pull request on my testsite, but my testsite wasn't set up correctly. I tried to fix it, but it still didn't work after getting all the relative_root_paths in place. I'm going to try again to set up the testsite and will be back after a little while. Thanks for your contributions! We'll get this figured out. I'm going to re-check the other pages that used this solution in case we are missing something subtle.

hoytpr commented 5 years ago

@tstilwel I think you did every correct, except you didn't include the new file base_path.html in your PR. See: https://github.com/datacarpentry/python-ecology-lesson/commit/39ed62b9b218242775b043e9eff3e76bb430602d#diff-f82a4c1c005f3128094642254b554892 You should be able to just copy that file. I'm going to set up a testsite so we can try again. Hope you're having fun!

hoytpr commented 5 years ago

@tstilwel , sorry this has been so much work, but my testsite is set up properly now at: https://github.com/hoytpr/testsite2 and if you can push that PR there this time we'll know if it worked. If you can include the base_path.html file, that would be great. If it works at the testsite, you can send the same PR to this site. Thanks again for your help! Pete

tstilwel commented 5 years ago

Ah, good find! No worries at all. I'll push the change shortly.

tstilwel commented 5 years ago

I was using a command to script the page.root changes but not all changes were the same. I followed the other PR closely so I think this should do it. Could you test that now?

I should probably learn how to setup a test site so I can validate this myself going forward.

hoytpr commented 5 years ago

Strange, but it didn't fix the problem. I'm not sure how to proceed at this point. This will have to wait until tomorrow now. I have a couple of fires at work to put out.

Thanks @tstilwel , if this is your first attempt at a PR, it's the best one I've ever seen. Peter

hoytpr commented 5 years ago

@tstilwel , I worked a few days and asked some questions, and have submitted my own PR for this. The resolution is much simpler than changing all the 'page.root' in the entire lesson. Rather just getting the proper YAML headers into the files.

tstilwel commented 5 years ago

Sounds great. I'll checkout your PR for changes. Should I go ahead and close this one?

Thanks for your time, Pete!

hoytpr commented 5 years ago

Yes, please close this for now.