Skip to content
This repository has been archived by the owner on Jul 7, 2023. It is now read-only.

Executable bit #486

Closed
wants to merge 6 commits into from
Closed

Conversation

martinpopel
Copy link
Contributor

Reintroducing the +x bit, which somehow got lost in some previous version.

Ryan Sepassi and others added 6 commits December 21, 2017 15:46
PiperOrigin-RevId: 179871302
…odel.body returns training loss

PiperOrigin-RevId: 179882031
PiperOrigin-RevId: 179882966
PiperOrigin-RevId: 179886783
@martinpopel martinpopel mentioned this pull request Dec 22, 2017
@rsepassi
Copy link
Contributor

@martinpopel What does adding the executable bit buy you? It seems that the Travis tests are fine. Is it just cosmetic?

The reason I ask is because these files are auto-generated when we export out of our internal repo out to GitHub and it's likely that the bit will be lost again unless I manually add it back every time. Yes, it'd be nice to have it automated somehow but that would require some eng work that I'd rather not do if I don't have to.

@martinpopel
Copy link
Contributor Author

When testing a new version (e.g. an unmerged PR) from git, I am used just to add the repo to PYTHONPATH and the bin/ to PATH, but for this I need the scripts to be executable.
I had this in previous versions of my checkout, but now I am not sure if it was executable also on github.

I know that when installed with pip, the executable bit is set by pip for the installed scripts, so this does not effect most users. Still I think it is a common practice to store the Python scripts executable in git.

BTW: I understand you want to allow importing also the t2t-* scripts as libraries and thus you duplicate the code to *.py, but this seems like non-standard solution. The common solution is that the script is just a thin wrapper and all the important code (which could be useful for importing) is stored elsewhere (in standard *.py modules).

@rsepassi
Copy link
Contributor

Ah, I see.

Until I can figure out a way of getting things to play nice during our export out to GitHub, let's leave off the x bit. You can pip install . (or pip install -e . if you want it to pick up edits) from within the repo if you're testing something from a PR (which is what I do locally as well).

Agreed that copying the whole file is not ideal, and I'll see if I can clean it up.

@rsepassi rsepassi closed this Dec 22, 2017
@martinpopel martinpopel deleted the executable-bit branch December 22, 2017 18:13
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants