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

Images in "Walled Garden" blogpost #3

Closed
ssddanbrown opened this issue Mar 4, 2022 · 10 comments
Closed

Images in "Walled Garden" blogpost #3

ssddanbrown opened this issue Mar 4, 2022 · 10 comments

Comments

@ssddanbrown
Copy link
Collaborator

ssddanbrown commented Mar 4, 2022

I have a branch here: https://github.com/ssddanbrown/owa-website-lfs/tree/walled_garden_images

Within that I have a commit where I've:

  • Extracted the images from the PDF, renamed, and added them to the repo using Git LFS.
  • Updated the "Walled Garden" blogpost with added markdown image references to the image files, with custom written descriptive alt text for accessibility.

There's also a few changes due to adding Git LFS support (logo file change and .gitattributes file for identifying LFS files).

I didn't pay attention to much to the formatting at this time. Will likely need some adjusting to fit/arrange images but didn't want to wonder outside a specific scope at this time, especially as much of the styles are yet to be defined. Some of the images may also need a little cleaning up where they might have been masked/cropped in the PDF.

I've opened this issue, rather than offer a PR, due to one of the most annoying limitations with Git LFS: You can't push git LFS files to a public GitHub fork, only those with write access to the original repo can. Public users/forks should still be able to pull these down fine and push (Non-LFS) changes to public forks. Since I don't have write access to this repo I've therefore pushed to a detched-fork repo. This still could be added as a remote locally, merged by someone (with write access) locally then pushed back up here.

If you don't want to go down the LFS path, just let me know the approach you'd want to take and I can change & re-submit things.

@capjamesg
Copy link
Contributor

@ssddanbrown Thank you for your contributions! I copied all of your commit changes into a new commit. I had no idea there was that limitation for Git LFS and merging commits. I don't think we'll run into that edge case again :)

@ssddanbrown
Copy link
Collaborator Author

Happy to help and thanks for including my changes! Will therefore close this off.

I don't think we'll run into that edge case again :)

Might do if someone wants to add images. Guess they could always just provide images separate (Uploaded to issue/PR via GitHub) for a maintainer to merge. Or create a separated fork as I did.

If provided as a separate fork, you should still be able to officially merge that into the main repo locally (Instead of copying files) using git (Add other repo as another origin, fetch branch from that origin, merge) which would keep contribution and authorship information. I'm not fussed in this case but can sometimes be important.

@capjamesg
Copy link
Contributor

That is a good call @ssddanbrown. I don't have much experience with merging across separate forks using the Git command line, hence my approach. I'll look into this further.

@aliblackwell
Copy link
Collaborator

Hi @capjamesg if you add me to the repo I can fix the commit messages locally to reflect proper attribution, and force push them back up. I think it's important commits are properly attributed. If you want to have a go yourself, you want to do an interactive rebase following the instructions here: https://www.sitepoint.com/git-interactive-rebase-guide. You should be able to pluck @ssddanbrown's commit hash into the history, squashing your version of the commit.

@aliblackwell
Copy link
Collaborator

I have completed this process here: https://github.com/aliblackwell/owa-website

It is not a fork due to the issue above (I mean it was originally, but I ended up creating a new repo and pushing the cleaned history up).

I am doing some other bits on the project just to get into it (installing eleventyNavigation, putting pages into their own dir and using markdown), I will upload to this repo and then @capjamesg you can give me access to the repo and I'll force push this repo to overwrite what's there. Thanks!

@capjamesg
Copy link
Contributor

@aliblackwell That would be excellent. I have just given you access to the repository so you can make the requisite changes. I concur that full and proper attribution is critical. I haven't done enough rebasing to feel confident doing it on a repository with enough collaborators, although I feel inspired to set up a test playground to do so with a private repo on my personal account.

@capjamesg capjamesg reopened this Mar 4, 2022
@capjamesg
Copy link
Contributor

Let's leave this issue open until we fix the attribution :)

@aliblackwell
Copy link
Collaborator

That is resolved now, I have force pushed and the history has been re-written.

I have had some issues with Git LFS; I had to re-download all the images manually from @ssddanbrown's repository (right click, save) as when I tried to push it said I had some missing references, and when I went to preview the files on my computer, they didn't show any thumbnails. Downloading everything manually made the image thumbs appear again in my Finder application. A git status showed the files as modified, and then when I did git add -A everything went green - there was no need to commit. I was then able to push.

It might be worth doing a bit more testing of the repo (e.g. clone down, fresh install) to ensure the images don't drop out of people's file systems and prevent them from contributing in future. @ssddanbrown have you encountered anything like this before? I've attached a zip of all the images to this for future reference. Overwriting the images in the repo with these and then running git add -A has fixed it for me twice. But be good to know it just worked!

walled-gardens.zip

@ssddanbrown
Copy link
Collaborator Author

I had to re-download all the images manually from @ssddanbrown's repository (right click, save) as when I tried to push it said I had some missing references
@ssddanbrown have you encountered anything like this before?

Sounds like you just had the pointers to the files, as if git-lfs had not pulled down the files yet.
Can sometimes occur, maybe if lfs is not setup or if you have the repo from before lfs was used.
Think you should be able to use git lfs pull in these scenarios to tell LFS to ensure the require large files are present.

I did just do a quick test of cloning the repo down fresh and that seemed to work as desired, with the images provided, just on the git clone.

@ssddanbrown
Copy link
Collaborator Author

Since the original work has been merged with attribution I'll close this off now.

I've opened PR #13 to expand upon, and provide clarity, with regard to git-lfs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants