Wrong date used for last commit date about jamstackthemes HOT 6 CLOSED

I'm working on modifying the generate-github.js script now.

One issue I'm running into is that if the branch is not set explicitly in the theme's YAML front matter then "master" is assumed as the default, but master is not always present in the repo (example) which then results in a 404 when making the call to the github API branches endpoint for getting the last commit date for the branch.

This can be dealt with in a couple of ways:

1. Ignore the theme, report/log it, but still allow the data/themes.json file to be generated for the other themes
2. Instead of assuming "master" when no branch is specified, use the default_branch property from the main repo object that was retrieved in the initial get repos call.

It is easy enough to go with option 2, but that would assume the default branch is the correct one to use, which seems like it might be reasonable though there is something to be said for having the branch set explicitly to the correct one in the front matter. What do you think?

from jamstackthemes.

I think number 2 seems like the correct approach. I imagine most themes default_branch property is still set to master right?

from jamstackthemes.

Yes, for the most part the default_branch is master, however there were around 56 repos that did not have a master branch. I didn't go through them all but the handful I manually examined seemed to forgo the "master" branch in favor of a "gh-pages" branch as the default branch.

I'll continue making the modifications incorporating option 2 from above.

from jamstackthemes.

A quick update on this issue. I'm running into a few problems with this because throughout some of the templates there are checks for Params.github_branch which comes from the theme's front matter, which if not present is being defaulted to "master". That default won't be correct for the themes that do not have a master branch.

For instance, in index.html there is this...

    {{ $repoName := printf "%s-%s" (substr (replace .Params.github "/" "-") 19 | urlize) (cond (ne .Params.github_branch nil) .Params.github_branch "master") }}
    {{ $repo := index .Site.Data.themes $repoName }}

...and for a repo like github.com/Phlow/feeling-responsive where there is no master branch in the above $repo never gets set because the wrong $repoName is being constructed when the branch is defaulted to "master" since no github_branch is set in that theme's front matter.

The scrub script that you mentioned in your comment on issue #12 might be the key to solving this. In addition to the"stale" property that is to be added, if the github_branch is updated as well with the correct branch from data/themes.json (which as part of this change is now retrieved and set from the github api using the default_branch property of the repo) then the problem will resolve itself.

I believe the scrub script would need to be changed in how it's driving the processing—right now the main loop is being driven by the files present in the content/theme directory, but I think it will need to be driven by the keys in the object present in the data/themes.json file. The reason being the branch name is part of the themeKey that gets built and there's no way to reconstruct it properly from the front matter alone (since for themes without a branch name specified it cannot be defaulted to master).

I can take a crack at the scrub script but wanted to check with you first for, 1) your thoughts on the above and 2) to ensure you're not also working on the scrub script.

from jamstackthemes.

Yes the scrub script can almost be completey re-written, it was not ready at all. I am not working on it.

Your right the scrub script needs its main loop to run off data/themes.json. Reading the branch name from data/themes.json is the way forward.

We still need to support people optionally declaring their branch name in the front-matter.

from jamstackthemes.

Looks like this was handled in commit 987e109 so have closed the issue.

Sorry, I really should have treated #42 and this as one issue/commit since they were very tightly related. I think I was confused due to my misunderstanding of where the stale property was going to reside.

from jamstackthemes.