Skip to content
This repository was archived by the owner on Nov 6, 2023. It is now read-only.

Some trivial enhancements#256

Closed
kirelagin wants to merge 6 commits intoEFForg:masterfrom
kirelagin:enhance
Closed

Some trivial enhancements#256
kirelagin wants to merge 6 commits intoEFForg:masterfrom
kirelagin:enhance

Conversation

@kirelagin
Copy link
Copy Markdown
Contributor

@kirelagin kirelagin commented Apr 27, 2014

No description provided.

diracdeltas
diracdeltas reviewed Apr 28, 2014
View reviewed changes
makecrx.sh Outdated
Copy link
Copy Markdown
Contributor

@diracdeltas diracdeltas Apr 28, 2014

Choose a reason for hiding this comment

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

I used shasum instead of sha1sum for OS X compatibility. (Please check that your changes work with OS X if you can.)

@kirelagin
Copy link
Copy Markdown
Contributor Author

kirelagin commented Apr 28, 2014

Ah, right, OS X. Actually, the only potentially bad commit is exactly the one about sha1sum.
OS X has openssl by default, right? I'll update the pull request to use it then.

@kirelagin
Copy link
Copy Markdown
Contributor Author

kirelagin commented Apr 28, 2014

As I can see sha1 hash is actually just echoed and not used elsewhere. So I assume the format doesn't really matter and it's ok if it won't output the file name, right?

% shasum /tmp/foo 
da39a3ee5e6b4b0d3255bfef95601890afd80709  /tmp/foo

% openssl dgst -sha1 /tmp/foo | cut -d ' ' -f 2
da39a3ee5e6b4b0d3255bfef95601890afd80709

@diracdeltas
Copy link
Copy Markdown
Contributor

diracdeltas commented Apr 28, 2014

Yep, format doesn't matter.

Also, see this discussion about python env: #138.

@pde
Copy link
Copy Markdown
Contributor

pde commented Jul 25, 2014

@diracdeltas should we merge this?

jsha
jsha reviewed Aug 14, 2014
View reviewed changes
makexpi.sh
Copy link
Copy Markdown
Member

@jsha jsha Aug 14, 2014

Choose a reason for hiding this comment

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

I believe the purpose of this line is to allow using a recent makexpi to build an older release, but I'm not sure. @pde, do you know?

In any case, please leave the python2.7 call as-is. There was an earlier issue filed where someone explained how it's slightly more secure to call python explicitly when you can.

@jsha
Copy link
Copy Markdown
Member

jsha commented Dec 17, 2014

Makexpi has been heavily modified since this change so it's no longer applicable. Thank you for sending, though, and sorry we couldn't merge it earlier.

@jsha jsha closed this Dec 17, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Assignees

No one assigned

Labels

enhancement needs revision

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@kirelagin @diracdeltas @pde @jsha