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

Allow fetching special files in the documentation #1698

Merged

Conversation

EWright212
Copy link
Contributor

@EWright212 EWright212 commented Jul 6, 2020

In #1514, we added a symlink so that the docs would show up on the demo app …but
symlinks aren't supported on Windows and was causing a problem when trying to bundle.

This instead adds a lookup table of files not in docs/ which allows us to render those.

This partially fixes #1693, in allowing Administrate itself to be bundled without a symlink but
other dependencies have the same problem.

Copy link
Member

@nickcharlton nickcharlton left a comment

Choose a reason for hiding this comment

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

Nice! A few comments to make it a little bit more Ruby-like.

I think too we might want to look at trying to test this out in: spec/features/documentation_spec.rb

spec/example_app/app/controllers/docs_controller.rb Outdated Show resolved Hide resolved
spec/example_app/app/controllers/docs_controller.rb Outdated Show resolved Hide resolved
@hound hound bot deleted a comment from nickcharlton Jul 8, 2020
@hound hound bot deleted a comment from nickcharlton Jul 8, 2020
Copy link
Collaborator

@pablobm pablobm left a comment

Choose a reason for hiding this comment

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

I'm trying to test this, but I'm having issues getting things installed on Windows (surprise). I'm trying something new; I'll report back.

spec/example_app/app/controllers/docs_controller.rb Outdated Show resolved Hide resolved
This reverts commit 1ed0804.

Removed commits that make README a special file, as they belong in their own pull request.

Fixed issue where symlink doesn't work on Windows machines. Removed symlink, instead load the Contributing.md doc via a Special File hash in the Docs Controller Class.

Co-author: Kehinde Olofinmoyin <kehindeolofinmoyin@hotmail.com>"
@pablobm
Copy link
Collaborator

pablobm commented Jul 11, 2020

As mentioned, I've been trying to get this working on Windows, but have failed so far :-/ How did you install things on Windows yourself? I have tried this so far:

  • RailsInstaller: latest version of Ruby there is 2.3, and Administrate requires at least 2.4.
  • WSL+Ubuntu on Windows: symlinks appear to work.
  • RubyInstaller: I can't get bundle install to complete, first for lack of symlinks, then due to the gem kgio not being compatible.

So it appears that you installed things in some way that allowed you to run bundle install, but yet failed later due to lack of symlinks. Am I correct?

Copy link
Member

@nickcharlton nickcharlton 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 is good to go now.

To answer @pablobm's question, if we flip the messaging of this PR to be about what it really is, vs. what motivated it is perhaps a better way of putting it. We were trying to fix #1693, where Administrate was using a symlink and breaking things. I don't think we can do much about dependencies (apart from going to their repos and opening PRs to fix them there).

We could perhaps rename this to be: "Allow fetching special files in the documentation", with a body like:

In #1514, we added a symlink so that the docs would show up on the demo app …but
symlinks aren't supported on Windows and was causing a problem when trying to bundle.

This instead adds a lookup table of files not in docs/ which allows us to render those.

This partially fixes #1693, in allowing Administrate itself to be bundled without a symlink but
other dependencies have the same problem.

spec/example_app/app/controllers/docs_controller.rb Outdated Show resolved Hide resolved
spec/example_app/app/controllers/docs_controller.rb Outdated Show resolved Hide resolved
@pablobm
Copy link
Collaborator

pablobm commented Jul 23, 2020

Right, I had misunderstood something. I thought this was about running the Administrate dev environment, which would require running bundle install in the folder of the Administrate gem. However, this is about using Administrate in a different project. I can reproduce this now.

OK, let's try this. I can't really test it properly because the problem doesn't appear if I install the gem from the GitHub repo (gem 'administrate', git: '...') as opposed to from the gem proper (just gem 'administrate'). The error does point at that symlink though, so I think it's worth trying this.

EWright212 and others added 2 commits July 31, 2020 11:06
Co-authored-by: Nick Charlton <nick@nickcharlton.net>
Co-authored-by: Nick Charlton <nick@nickcharlton.net>
@EWright212 EWright212 changed the title Ew fix windows symlink issue Allow fetching special files in the documentation Jul 31, 2020
@nickcharlton nickcharlton merged commit 40e0652 into thoughtbot:master Jul 31, 2020
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

Successfully merging this pull request may close these issues.

Unable to bundle on Windows 10 because of a symlink.
3 participants