Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings
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 into
EFForg:masterEFForg/https-everywhere:masterfrom
kirelagin:enhancekirelagin/https-everywhere:enhanceCopy head branch name to clipboard
Closed

Some trivial enhancements#256
kirelagin wants to merge 6 commits into
EFForg:masterEFForg/https-everywhere:masterfrom
kirelagin:enhancekirelagin/https-everywhere:enhanceCopy head branch name to clipboard

Conversation

@kirelagin

@kirelagin kirelagin commented Apr 27, 2014

Copy link
Copy Markdown
Contributor

No description provided.

diracdeltas
diracdeltas reviewed Apr 28, 2014
View reviewed changes
Comment thread makecrx.sh Outdated

@diracdeltas diracdeltas Apr 28, 2014

Copy link
Copy Markdown
Contributor

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

kirelagin commented Apr 28, 2014

Copy link
Copy Markdown
Contributor Author

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

kirelagin commented Apr 28, 2014

Copy link
Copy Markdown
Contributor Author

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

diracdeltas commented Apr 28, 2014

Copy link
Copy Markdown
Contributor

Yep, format doesn't matter.

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

@pde

pde commented Jul 25, 2014

Copy link
Copy Markdown
Contributor

@diracdeltas should we merge this?

jsha
jsha reviewed Aug 14, 2014
View reviewed changes
Comment thread makexpi.sh

@jsha jsha Aug 14, 2014

Copy link
Copy Markdown
Member

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

jsha commented Dec 17, 2014

Copy link
Copy Markdown
Member

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
Morty Proxy This is a proxified and sanitized view of the page, visit original site.