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

ensure_git_status_clean documentation confusing wording for ignored parameter #19818

Closed
simondelphia opened this issue Jan 19, 2022 · 16 comments · Fixed by #20976
Closed

ensure_git_status_clean documentation confusing wording for ignored parameter #19818

simondelphia opened this issue Jan 19, 2022 · 16 comments · Fixed by #20976
Assignees

Comments

@simondelphia
Copy link

https://docs.fastlane.tools/actions/ensure_git_status_clean/

For the parameter ignored the description says "The flag whether to ignore file the git status if the repo is dirty"

I'm not clear on what that means. Looks like a typo.

Thank you

@fastlane-bot
Copy link

It seems like you have not included the output of fastlane env
To make it easier for us help you resolve this issue, please update the issue to include the output of fastlane env 👍

@lucgrabowski
Copy link
Contributor

It looks like you're right, it's not a bool flag, it's a string parameter. git status docs should be helpful to understand how to use it

class EnsureGitStatusCleanAction < Action
def self.run(params)
if params[:ignored]
ignored_file = params[:ignored]
repo_status = Actions.sh("git status --porcelain --ignored #{ignored_file}")
else
repo_status = Actions.sh("git status --porcelain")
end

@fastlane fastlane deleted a comment Jan 22, 2022
@tcf909
Copy link

tcf909 commented Feb 8, 2022

Actually, even worse is the actual command being run is wrong.

It needs to be:

git status --porcelain --ignored=#{ignored_file}

if you run it without the equals sign it basically always comes back as empty because it is using "no" as the ... for the git-status command.

I would consider this a show stopper (critical). The consequence: if you use it with reset_git_repo you will lose all uncommitted changes because the gate of a block being (ensure_git_status_clean) will always allow the process to flow through it ie:

ensure_git_status_clean(
  ignored: 'no'
)

begin

  ...

ensure

  reset_git_repo(
    disregard_gitignore: false,
  )

  # Potentially all uncommitted files lost now because ensure_git_status_clean did not raise correctly

end

@fastlane-bot
Copy link

There hasn't been any activity on this issue recently. Due to the high number of incoming GitHub notifications, we have to clean some of the old issues, as many of them have already been resolved with the latest updates.

Please make sure to update to the latest fastlane version and check if that solves the issue. Let us know if that works for you by adding a comment 👍

Friendly reminder: contributions are always welcome! Check out CONTRIBUTING.md for more information on how to help with fastlane and feel free to tackle this issue yourself 💪

This issue will be auto-closed if there is no reply within 1 month.

@simondelphia
Copy link
Author

Still an issue

@fastlane-bot
Copy link

There hasn't been any activity on this issue recently. Due to the high number of incoming GitHub notifications, we have to clean some of the old issues, as many of them have already been resolved with the latest updates.

Please make sure to update to the latest fastlane version and check if that solves the issue. Let us know if that works for you by adding a comment 👍

Friendly reminder: contributions are always welcome! Check out CONTRIBUTING.md for more information on how to help with fastlane and feel free to tackle this issue yourself 💪

This issue will be auto-closed if there is no reply within 1 month.

@simondelphia
Copy link
Author

Still an issue

@fastlane-bot
Copy link

There hasn't been any activity on this issue recently. Due to the high number of incoming GitHub notifications, we have to clean some of the old issues, as many of them have already been resolved with the latest updates.

Please make sure to update to the latest fastlane version and check if that solves the issue. Let us know if that works for you by adding a comment 👍

Friendly reminder: contributions are always welcome! Check out CONTRIBUTING.md for more information on how to help with fastlane and feel free to tackle this issue yourself 💪

This issue will be auto-closed if there is no reply within 1 month.

@simondelphia
Copy link
Author

still an issue

@Ryabchikus
Copy link

I'm here too
I can't exclude some files from git check

@fastlane-bot
Copy link

There hasn't been any activity on this issue recently. Due to the high number of incoming GitHub notifications, we have to clean some of the old issues, as many of them have already been resolved with the latest updates.

Please make sure to update to the latest fastlane version and check if that solves the issue. Let us know if that works for you by adding a comment 👍

Friendly reminder: contributions are always welcome! Check out CONTRIBUTING.md for more information on how to help with fastlane and feel free to tackle this issue yourself 💪

This issue will be auto-closed if there is no reply within 1 month.

@simondelphia
Copy link
Author

still an issue

@ObjectiveCesar
Copy link

ObjectiveCesar commented Jan 17, 2023

And still an issue.

The PR to at least fix the documentation issue is open here: #20976

But imho the underlaying issue still persists.

@revolter
Copy link
Collaborator

I can rename that PR and also fix the issue. Would you be willing to test it? As I'm not currently using that action.

@revolter revolter self-assigned this Jan 17, 2023
@revolter revolter added this to the Improve a fastlane action milestone Jan 17, 2023
@ObjectiveCesar
Copy link

I can rename that PR and also fix the issue. Would you be willing to test it? As I'm not currently using that action.

I could try at least. Never built fastlane from source so far. :)

@revolter
Copy link
Collaborator

You don't have to 😁 I added the testing steps in the PR description

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 a pull request may close this issue.

7 participants