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

Attempt to sort licenses in list #164

Merged
merged 1 commit into from Dec 18, 2013
Merged

Attempt to sort licenses in list #164

merged 1 commit into from Dec 18, 2013

Conversation

afeld
Copy link
Contributor

@afeld afeld commented Dec 18, 2013

Not quite sure why it is different in prod than dev (though I remember coming across issues w/ sort inconsistency before), but this should do the trick. Fixes #162 (comment). See also: jekyll/jekyll#1802 (comment)

/cc @XhmikosR

@XhmikosR
Copy link
Contributor

Nice workaround @afeld! Just out of curiosity, how are currently sorted?

I tried the patch and works the same as before on my dev environment; it sorts the licenses by name which is the path too. So hopefully it will fix the issue for the gh-pages too.

@afeld
Copy link
Contributor Author

afeld commented Dec 18, 2013

how are currently sorted?

I think it's due to how the files are being pulled in in this method. Possibly due to inconsistencies btw OSes?

@XhmikosR
Copy link
Contributor

I thought about something being OS related but all pages in /licenses are lowercase already. Oh well, as long as it works right now.

Feel free to merge this since it seems to work fine here.

afeld added a commit that referenced this pull request Dec 18, 2013
Attempt to sort licenses in list
@afeld afeld merged commit 4ac4a1e into gh-pages Dec 18, 2013
@afeld
Copy link
Contributor Author

afeld commented Dec 18, 2013

Works on http://choosealicense.com/licenses/ 😃

@afeld afeld deleted the sort-licenses branch December 18, 2013 08:42
@XhmikosR
Copy link
Contributor

Perfect!

@afeld
Copy link
Contributor Author

afeld commented Dec 18, 2013

P.S. funny that the previous sort order of the "Featured licenses" happened to match the order on the front page.

old home page

@benbalter
Copy link
Contributor

So to clarify, this sorts the non-featured license alphabetically?

@benbalter
Copy link
Contributor

funny that the previous sort order of the "Featured licenses" happened to match the order on the front page.

That was intentional. Is it possible to unsort the featured licenses? Really think the first license on the page should be MIT, not Apache. See generally, http://www.youtube.com/watch?v=-bAAlPXB2-c

@XhmikosR
Copy link
Contributor

@benbalter: it might have worked for github pages but on Windows I got the result you see in the screenshots...

You could do

 licenses.html | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/licenses.html b/licenses.html
index 0445a54..6ad7bd0 100644
--- a/licenses.html
+++ b/licenses.html
@@ -7,9 +7,7 @@ title: Licenses

 <h2>Featured Licenses</h2>

-{% assign sorted_pages = site.pages | sort: 'path' %}
-
-{% for page in sorted_pages %}
+{% for page in site.pages %}
   {% if page.layout == "license" %}
     {% if page.featured %}
       {% include license-overview.html %}
@@ -23,6 +21,8 @@ title: Licenses
   community. For example, Perl developers often choose the Artistic License.
 </p>

+{% assign sorted_pages = site.pages | sort: 'path' %}
+
 {% for page in sorted_pages %}
   {% if page.layout == "license" %}
     {% if page.featured != true and page.hide-from-license-list != true %}

but the order in the featured licenses won't be consistent again.

@afeld
Copy link
Contributor Author

afeld commented Dec 18, 2013

@benbalter Sorry for not waiting to hear from you! @XhmikosR's quick-fix would work, but would be better to make the sort order explicit, rather than just rely on the fluke.

@XhmikosR
Copy link
Contributor

It should be possible to make the featured licenses' order explicit since they are only 3 at this point but I'm not sure it's worth the hassle...

I have no opinion on the subject as long as the output is the same regardless of the OS :)

@XhmikosR
Copy link
Contributor

BTW, can someone give me a tip on how to make github pages actually ignore the CNAME and publish the pages? Apart from me deleting the file. This is a feature I'd like to see because when there's a CNAME file the fork can only be used to develop locally.

@afeld
Copy link
Contributor Author

afeld commented Dec 18, 2013

@XhmikosR The Pages URL will work even if you have a CNAME file (e.g. http://afeld.github.io/hackerhours.org/)... you just need to push a new commit on the gh-pages on your fork to trigger the page build.

@XhmikosR
Copy link
Contributor

@afeld But when I push to the gh-pages branch then I get the error that CNAME is already taken...

An example I just pushed to test this is http://xhmikosr.github.io/government.github.com/

@afeld
Copy link
Contributor Author

afeld commented Dec 18, 2013

Ah ok. In that case, I'd do the changes in a feature branch, then merge those into your gh-pages branch (which has CNAME removed) to test.

@XhmikosR
Copy link
Contributor

I tried it and only the html files are loaded. All other resources like CSS and JS return 404. So it still fails it just sort of works because there's index.html in the gh-pages branch. But I don't get any error in the repo settings.

@afeld
Copy link
Contributor Author

afeld commented Dec 18, 2013

Ha, that's because all of those asset URLs are absolute (/js/...), but your site is being served from a sub-path (/government.github.com/). Options now are to a) set up your own custom domain so that the site is served from /, or to do some Jekyll/Liquid hackery to make all those URLs relative. Wish I had a better solution for ya 😕

@XhmikosR
Copy link
Contributor

Ah too bad :( I wish there was an easier way to be able to test things in real life rather than my limited 1280x900 virtual machine...

@afeld
Copy link
Contributor Author

afeld commented Dec 19, 2013

Page order inconsistency addressed here: jekyll/jekyll#1848

@afeld
Copy link
Contributor Author

afeld commented Dec 19, 2013

PR to enable Jekyll to sort by custom properties: jekyll/jekyll#1849

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.

None yet

3 participants