jekyll / Utterson

CI benchmarking suite for Jekyll
MIT License
13 stars 8 forks source link

Use the site's basename as source directory #44

Closed ashmaroli closed 6 years ago

ashmaroli commented 6 years ago

It would be easier to gauge which site is being sampled currently while running ./bench locally

pathawks commented 6 years ago

This makes sense to me :+1:

pathawks commented 6 years ago

With this change, the MD5 hash isn't being used for anything anymore, is it?

I can't exactly remember what my thinking there was anyway. I think I was trying to obfuscate which site was being built to focus on the numbers rather than the individual site, but I don't know that it makes much difference. There have been a couple times when I've noticed a build issue with a site (as a result of building with --trace) and opened a PR for that site, so obviously it was never blind to begin with 😁

Edit: Thinking back, I think I wanted to avoid collisions when testing two repositories that had the same basename. If we used a hash instead of the basename, we could easily test forks and whatnot.

ashmaroli commented 6 years ago

the MD5 hash isn't being used for anything anymore

I wasn't sure.. so stashed it in a variable.. :stuck_out_tongue: Should I remove it?

pathawks commented 6 years ago

Go ahead and remove it, and also the check for which MD5 to use.

https://github.com/jekyll/Utterson/blob/d24ee3e8a678fc0bdd91d3b69dd4a330f5347be3/bench#L37-L41

ashmaroli commented 6 years ago

I also had a hard time making sense out of DESTINATION=${SOURCE/source/destination}.. :thinking: its not a string and the desired target is two subfolders deeper..?

pathawks commented 6 years ago
DESTINATION=${SOURCE/source/destination}

In SOURCE, replace every occurrence of source with destination

From the Bash Reference:

${parameter/pattern/string}

The pattern is expanded to produce a pattern just as in filename expansion. Parameter is expanded and the longest match of pattern against its value is replaced with string. If pattern begins with β€˜/’, all matches of pattern are replaced with string. Normally only the first match is replaced. If pattern begins with β€˜#’, it must match at the beginning of the expanded value of parameter. If pattern begins with β€˜%’, it must match at the end of the expanded value of parameter. If string is null, matches of pattern are deleted and the / following pattern may be omitted. If the nocasematch shell option (see the description of shopt in The Shopt Builtin) is enabled, the match is performed without regard to the case of alphabetic characters. If parameter is β€˜@’ or β€˜β€™, the substitution operation is applied to each positional parameter in turn, and the expansion is the resultant list. If parameter is an array variable subscripted with β€˜@’ or β€˜β€™, the substitution operation is applied to each member of the array in turn, and the expansion is the resultant list.

ashmaroli commented 6 years ago

Thank you very much! The note helps loads. TIL :slightly_smiling_face: :

pathawks commented 6 years ago

Yea, unfortunately there’s some pretty heavy Bash black magic going on here. Might be easier for the Jekyll community to contribute if we could rewrite this in Ruby.

pathawks commented 6 years ago

This has been merged, but GitHub web interface doesn't seem to recognize that πŸ˜•

pathawks commented 6 years ago

Also: :tada: This repo now has more than one contributor! :tada: