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

Add docs site shell #2739

Merged
merged 1 commit into from
May 24, 2018
Merged

Add docs site shell #2739

merged 1 commit into from
May 24, 2018

Conversation

jgayfer
Copy link

@jgayfer jgayfer commented May 18, 2018

This PR contains the shell of a static site built to house the Solidus developers documentation.

A working version of the site can be viewed at: http://jgayfer.s3-website.us-east-1.amazonaws.com/

This site is modelled after the new Solidus marketing site to ensure a consistent user experience across both sites.

This site could also be used for future Solidus docs sites (API and Admin) to further the goal of a unified Solidus experience on the web.

Rationale

Keeping the source for the docs site within the Solidus repository has several advantages:

  • It allows the docs to be versioned alongside the code. When major changes are introduced in Solidus, it is clear where and how the docs should also be changed.
  • Having the docs stay within the Solidus repository ensures that they will be evaluated (by the community and the core developer team) at the same high standard that Solidus code contributions are.
  • There's no risk in adding the docs site to the main Solidus repository. The docs are self-contained and don't affect the code base. And, for the forseeable future, they don't significantly increase the size of git clones of Solidus.

We can decide to move the docs to another repository in the future. But first, we would need to create a plan to make sure the docs aren't treated as a second-class citizen.

Next steps

Note that this PR is a shell for the docs site:

  • In a later PR, we would need to move the docs to docs_site/source/developers directory.
  • We would need to change each article extension from .md to .html.md, as this is the format Middleman requires.
  • We would also need to go through each article and change each relative link from .md to .html.
  • Regarding deployment, this site is setup so that it can easily be deployed to an AWS S3 bucket using the middleman-s3_sync gem.

)
end
end
end

Choose a reason for hiding this comment

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

Layout/TrailingBlankLines: Final newline missing.

file = File.open(asset, 'r') { |f| f.read }
# we pass svg-targeting css classes through here, ex. .svg-color--blue. The class targets fill, stroke, poly, circle, etc.
css_class = options[:class]
aspect_ratio = options[:preserveAspectRatio]

Choose a reason for hiding this comment

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

Lint/UselessAssignment: Useless assignment to variable - aspect_ratio.

def inline_svg(filename, options = {})
asset = "source/assets/images/#{filename}"

if File.exists?(asset)

Choose a reason for hiding this comment

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

Lint/DeprecatedClassMethods: File.exists? is deprecated in favor of File.exist?.

@@ -0,0 +1,51 @@
# Middleman - Inline SVG Helper

Choose a reason for hiding this comment

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

Style/FrozenStringLiteralComment: Missing magic comment # frozen_string_literal: true.

cls = ""
end
end
return cls

Choose a reason for hiding this comment

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

Style/RedundantReturn: Redundant return detected.

def nav_inactive(paths)
cls = "no-active"
paths.each do |path|
if(current_page.path.start_with? path)

Choose a reason for hiding this comment

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

Layout/SpaceAroundKeyword: Space after keyword if is missing.

@@ -0,0 +1,37 @@
module CustomHelpers
def full_title(page_title=nil, site_title)

Choose a reason for hiding this comment

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

Style/OptionalArguments: Optional arguments should appear at the end of the argument list.
Layout/SpaceAroundEqualsInParameterDefault: Surrounding space missing in default value assignment.

@@ -0,0 +1,37 @@
module CustomHelpers

Choose a reason for hiding this comment

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

Style/FrozenStringLiteralComment: Missing magic comment # frozen_string_literal: true.


def discover_title(page = current_page)
page_title = current_page.data.title || retrieve_page_header(page)
category = page.path[/\/(.*?)\/.*\.html/, 1]&.gsub('-', ' ')&.capitalize

Choose a reason for hiding this comment

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

Lint/Syntax: unexpected token error

@@ -0,0 +1,8 @@
source "https://rubygems.org"

Choose a reason for hiding this comment

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

Style/FrozenStringLiteralComment: Missing magic comment # frozen_string_literal: true.

def nav_inactive(paths)
cls = "no-active"
paths.each do |path|
if (current_page.path.start_with? path)

Choose a reason for hiding this comment

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

Style/ParenthesesAroundCondition: Don't use parentheses around the condition of an if.

# frozen_string_literal: true

module CustomHelpers
def full_title(site_title, page_title=nil)

Choose a reason for hiding this comment

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

Layout/SpaceAroundEqualsInParameterDefault: Surrounding space missing in default value assignment.

@jgayfer jgayfer force-pushed the docs_site branch 2 times, most recently from 9049063 to ee8ebcf Compare May 18, 2018 22:16
@@ -0,0 +1,111 @@
---
title: Home
Copy link
Member

Choose a reason for hiding this comment

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

is this file needed?

Copy link
Author

Choose a reason for hiding this comment

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

@kennyadsl No, it's not. It was just a template/example of the styles we are using. I'll remove it!

Copy link
Member

@kennyadsl kennyadsl left a comment

Choose a reason for hiding this comment

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

I didn't see all the files but at the moment I agree we shouldn't move guides files to another repository. Also, the guides preview looks great.

@tvdeyen tvdeyen merged commit 146c69d into solidusio:master May 24, 2018
@tvdeyen
Copy link
Member

tvdeyen commented May 24, 2018

Thanks

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.

5 participants