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

Fix make exec-hash script #424

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

SidestreamStrongStrawberry
Copy link
Collaborator

@SidestreamStrongStrawberry SidestreamStrongStrawberry commented Sep 3, 2024

Closes #425

Context

  • Running script currently returns error on mac
date: illegal option -- d
usage: date [-jnRu] [-I[date|hours|minutes|seconds]] [-f input_fmt]
            [-r filename|seconds] [-v[+|-]val[y|m|w|d|H|M|S]]
            [[[[mm]dd]HH]MM[[cc]yy][.SS] | new_date] [+output_fmt]
  • Inconsistent exec-doc name
    • Executive Vote in exec doc name will make the script fail (Error: Executive vote not found) as the script was develop to work with Executive vote
  • Tool for platform-independent hash generation (spell retro from 2024-08-12)

Copy link
Contributor

@amusingaxl amusingaxl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left a couple of suggestions.

scripts/hash-exec-copy.py Show resolved Hide resolved
scripts/hash-exec-copy.py Show resolved Hide resolved
scripts/hash-exec-copy.py Outdated Show resolved Hide resolved
scripts/hash-exec-copy.py Show resolved Hide resolved
scripts/hash-exec-copy.py Show resolved Hide resolved
Comment on lines 70 to 73
f"Executive%20vote%20-%20{date.strftime('%B %d, %Y')}.md",
f"Executive%20Vote%20-%20{date.strftime('%B %d, %Y')}.md",
f"Executive%20vote%20-%20{date.strftime('%B %-d, %Y')}.md",
f"Executive%20Vote%20-%20{date.strftime('%B %-d, %Y')}.md"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@SidestreamStrongStrawberry Could you please provide 4 command examples to test those 4 edge cases (as I assume you did it yourself)?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Date script command bash command Hashes match Comment
2024-03-06 make exec-hash date="2024-03-06" cast keccak -- "$(wget 'https://raw.githubusercontent.com/makerdao/community/17c96c446ac3339c1f0b3081f6afffad26b7c508/governance/votes/Executive%20Vote%20-%20March%206,%202024.md' -q -O - 2>/dev/null)" Exec doc title: Executive Vote - March 6, 2024.md
2024-05-06 make exec-hash date="2024-05-06" cast keccak -- "$(wget 'https://raw.githubusercontent.com/makerdao/community/be059e35221d04f91d7e2ed978175784e2f45607/governance/votes/Executive%20vote%20-%20May%2006,%202024.md' -q -O - 2>/dev/null)" Exec doc title: Executive vote - May 06, 2024.md
2024-05-30 make exec-hash date="2024-05-30" cast keccak -- "$(wget 'https://raw.githubusercontent.com/makerdao/community/16588abd76fefe0e1276ab630033facc541f954d/governance/votes/Executive%20vote%20-%20May%2030,%202024.md' -q -O - 2>/dev/null)" Exec doc title: Executive vote - May 30, 2024.md
2023-02-08 make exec-hash date="2023-02-08" cast keccak -- "$(wget 'https://raw.githubusercontent.com/makerdao/community/856ebe853b251537c4bfddb7daa92f9f2800260f/governance/votes/Executive%20vote%20-%20February%208,%202023.md' -q -O - 2>/dev/null)" Exec doc title: Executive vote - February 8, 2023.md

Makefile Show resolved Hide resolved
Copy link
Contributor

@amusingaxl amusingaxl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor

@amusingaxl amusingaxl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@amusingaxl
Copy link
Contributor

I believe you need to URL-encode the formatted date for this to work prolerly.

@SidestreamStrongStrawberry
Copy link
Collaborator Author

I believe you need to URL-encode the formatted date for this to work prolerly.

This can be improved but I don't think it affects the result.
It looks like the doc was uploaded in the wrong folder and moved it now.

image

The hash results matches with url: https://raw.githubusercontent.com/makerdao/community/23d01e9ea8e102d959e7b9bd1586cb4c31a8b812/governance/votes/Executive%20vote%20-%20September%2013,%202024.md

Copy link
Contributor

@amusingaxl amusingaxl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reapproving.

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.

Fix make exec-hash script
3 participants