GispoCoding / master-training-data

Repo for Gispo training materials.
1 stars 0 forks source link

REPO: Dedicate branch for publishing GitHub Pages #28

Closed JuhoErvasti closed 4 months ago

JuhoErvasti commented 5 months ago

Don't merge yet! Requires extra manual steps to make changes live.

Resolves: #19 and #20.

jeeemil commented 5 months ago

Initial review made. Great work!Wow! I didn't spot any errors. Some questions:

What was the function of find_keys.js? I noticed it gets deleted but not sure what the purpose of it was?

And just to be clear the pre-deploy.yaml seems to be incorporated into the render-deploy.yaml right?

JuhoErvasti commented 5 months ago

Yes, the pre-deploy workflow used to dispatch the render-deploy or update-assets, but it seemed unnecessary to me because the render-deploy has to find the modified folders regardless so it was just an additional step that did mostly the same thing as in render-deploy.

We used to have a functionality to lock answers in a box which required entering a key to unlock. The idea was for the trainees to first try to figure out the answer to a question and the trainer would give the answer key only after a certain period of time. The plan was to use it with courses you have to write code in, f.e. the PostgreSQL courses / the QGIS expression course. It was only ever in use with the PostGIS course, but it turned out to not work that well so I removed it from that repo and this one.

The way it used to work was the key was composed from the title of each HTML document in a specific way that made it look random, but there actually was a certain arbitrary logic to it, which meant that the key could be checked against dynamically in each html document.

As far as I recall the find_keys.js was used specifically in a workflow that ran it in a Node.js environment and saved the keys for each exercise in the repo, so that the trainer could get them from there and also the keys would get updated any time the title of an exercise would change (which would likely affect the key as well). But it was never actually in use in this repo and I'd forgotten to delete it before this PR.

jeeemil commented 5 months ago

@JuhoErvasti Thanks for the clarifications! Sounds like a good improvement to drop the pre-deploy workflow!

The find_keys.js makes a lot more sense now! Good idea to drop it!

JuhoErvasti commented 5 months ago

@jeeemil Thank you for the review! I'll make these changes live probably next monday. This requires an update since the GR003 course was added after this PR and also I have to do some final tests to make sure everything works ok in the repo.