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

data: fix shellcheck warnings in snapd.sh.in #8520

Merged
merged 4 commits into from Jun 3, 2020

Conversation

jelovac
Copy link
Contributor

@jelovac jelovac commented Apr 18, 2020

While troubleshooting why snapd is not added to path in Linux Mint LMDE 4 I noticed some shellcheck warnings.

This tiny fix resolves them.

Update

Signed the CLA and added the git email address to the launchpad account.

Copy link
Collaborator

@bboozzoo bboozzoo left a comment

Choose a reason for hiding this comment

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

Thanks!

@mvo5 mvo5 changed the title Fixing shellcheck warnings. data: fix shellcheck warnings in snapd.sh.in Apr 20, 2020
Copy link
Contributor

@mvo5 mvo5 left a comment

Choose a reason for hiding this comment

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

Thank you! Nice catch

@anonymouse64
Copy link
Member

The CLA check seems to be broken:

Could not determine commit range: expected two parents, but got ['3cd9d3635dd4639b14a6eb8010ee607c05339952']
##[error]Process completed with exit code 1.

@jelovac
Copy link
Contributor Author

jelovac commented May 6, 2020

The CLA check seems to be broken:

Could not determine commit range: expected two parents, but got ['3cd9d3635dd4639b14a6eb8010ee607c05339952']
##[error]Process completed with exit code 1.

Indeed.

I signed the CLA after the creation of pull request, could be related.

@stolowski
Copy link
Contributor

The CLA check seems to be broken:

Could not determine commit range: expected two parents, but got ['3cd9d3635dd4639b14a6eb8010ee607c05339952']
##[error]Process completed with exit code 1.

Indeed.

I signed the CLA after the creation of pull request, could be related.

I think it is something else, the cla-check looks for two "parent ..." entries reported by git cat-file -p @, but for this PR there is only one. I suspect this might be because you haven't worked in a branch created from master, but simply pushed to your master directory (but maybe this explanation is nonsense...)

@bboozzoo
Copy link
Collaborator

bboozzoo commented May 6, 2020

Perhaps merging master to this branch and pushing will fix it.

@jelovac
Copy link
Contributor Author

jelovac commented May 6, 2020

The CLA check seems to be broken:

Could not determine commit range: expected two parents, but got ['3cd9d3635dd4639b14a6eb8010ee607c05339952']
##[error]Process completed with exit code 1.

Indeed.
I signed the CLA after the creation of pull request, could be related.

I think it is something else, the cla-check looks for two "parent ..." entries reported by git cat-file -p @, but for this PR there is only one. I suspect this might be because you haven't worked in a branch created from master, but simply pushed to your master directory (but maybe this explanation is nonsense...)

Yes, I didn't create another branch from master on the fork, I simply committed on the forked master and then created a pull request.

So what should I do? Merge the snapcore:master into my forked master and try to push again?

@mvo5
Copy link
Contributor

mvo5 commented May 6, 2020

@jelovac Please try this, if that does not work we will find another way to fix it :)

@stolowski
Copy link
Contributor

Yay, it worked :)

@jelovac
Copy link
Contributor Author

jelovac commented May 6, 2020

yeah :)

@bboozzoo
Copy link
Collaborator

bboozzoo commented May 7, 2020

@mvo5 we can merge this now, the test failures are unrelated.

@bboozzoo
Copy link
Collaborator

@jelovac can you merge master once more and push. Or please enable edits from maintainers, so that I can push the merge myself.

@bboozzoo bboozzoo added the Simple 😃 A small PR which can be reviewed quickly label May 28, 2020
@jelovac
Copy link
Contributor Author

jelovac commented May 28, 2020

@bboozzoo merged.

Option "Allow edits by maintainers" was already enabled.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Simple 😃 A small PR which can be reviewed quickly
Projects
None yet
6 participants