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

Chmod option #189

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Chmod option #189

wants to merge 4 commits into from

Conversation

jkstrauss
Copy link

Added an option for BFG that allows users to change executable mode based on a file glob. This was inspired by a branch that you did as a quick fix a while back, non-executable. However, I wanted to make it more flexible.

This is my first attempt at any Scala programming, so any comments are welcome. Also, I was not sure if you want every feature to have a test case. I can always make that later if you wish.

Feel free to keep the copyright for this. I am assuming I will still be the author on as far as the Git history is concerned.

@jkstrauss
Copy link
Author

@rtyley: Is there any chance that this could be merged?

@javabrett
Copy link
Contributor

@jkstrauss Whilst I don't doubt that this is a well-implemented and clever feature ... I do wonder whether it is the sort of thing that it is typically worth re-writing history over, and therefore adding to core BFG. Some sort of scripting or plugin option is perhaps needed here to accommodate custom features.

I say this because - rewriting Git history, with-or-without BFG is kindof a big deal ... all other clones and histories become instantly incompatible from the first change forwards. For something like fixing file-permissions, especially for chmod +x on scripts, I would be inclined to:

  • Fix this on all active/building branch tips with a simple chmod +x and commit.
  • If you ever needed to checkout and build a historical commit, and aren't prepared to branch and add a commit to fix the permissions, run an additional build-script to run the required chmod +x first.

Is there a compelling reason that you want to undergo a history-rewrite just to fix permissions? Or is this a case of "running a history-rewrite anyway, may as well fix these too"?

@jkstrauss
Copy link
Author

@javabrett I actually used it for my own purposes. I was importing history from CVS, and the default implementation makes all files executable. Since nobody ever saw it, I have no downside in making everything look good from the outset.

That said, if I want this feature, I can just use my fork, but others may be interested as well.

Also, is there any plan in the works for the script or plugin you mentioned?

@jkstrauss jkstrauss force-pushed the chmod-option branch 2 times, most recently from edb846c to 5de9449 Compare February 1, 2018 16:03
@jkstrauss
Copy link
Author

I tried rebasing blindly, i.e., without checking it first, and the CI build failed. For the moment, I am resetting it back to the original commit.

@jkstrauss
Copy link
Author

@javabrett I finally succeed rebasing. Is there any reason why none of the org.scalatest imports seem to work in IntellliJ with Scala plugin? I basically had to guess how to refactor the test to meet the new way of doing things. I cannot even find any of the scalatest when using CTRL+N to search for classes.

@javabrett
Copy link
Contributor

I don't use IntelliJ so I can't comment on if/why it can't work with scalatest.

@floris-xlx
Copy link

hashah loser

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.

3 participants