-
-
Notifications
You must be signed in to change notification settings - Fork 316
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
;feat: Sandstorm: Upgrade Sandstorm package with permissions #2102
Conversation
For the record: this comes from the recent #425 (comment) comment on the old #425 issue, and updates the hledger app on sandstorm to (a) modern sandstorm and (b) modern hledger (1.9.2 -> 1.31). You could mention the hledger bump in the commit message ? Great! I'm ready to merge when you are. |
PS begin the commit message with semicolon to bypass lengthy CI building. |
And maybe briefly mention the new permissions abilities - any Sandstorm hledger user docs that need updating ? |
Ah I didn't realize it was the commit message not the PR message that needed the semicolon. I will check our metadata/documentation stuff prior to merge here. |
6bbf136
to
6d1b423
Compare
I tried amending the commit description and it didn't help skip CI: Anyways, I also removed some legacy cruft from the description and updated the changelog. |
I would probably use private browsing tabs as a guest to avoid mixing/accumulating permissions, but these three URLs should let us test the same test ledger with different levels of access: Full access URL: https://ocdhost.sandcats.io/shared/inXFsQuY2d3Fz5556TaPYqmUHxOX_KKAx5fe4H5OvSP |
Yes, probably the force push confused it. Unfortunately I don't know how to reliably detect the recent commits in all situations on github. |
Thank you very much! |
I doubt your CI can test Sandstorm, so I'm going to go ahead and skip CI here. :)
This PR updates the base box and vagrant-spk configuration to a more modern standard and fixes a couple issues with the build. HLedger already appears to correctly configure permissions thanks to @zarybnicky. I need to go grab my signing key to get a new package built, but I will endeavor to get a grain setup with some test URLs with different permission levels for testing shortly. (You may wish to wait until I get this part complete before merging, in case we find a bug.)
Note that I used Hacktoberfest as my incentive to handle some long-overdue PRs, so while it's not required, if you want to give me credit, please add a
hacktoberfest-accepted
label to this PR.