Skip to content
This repository has been archived by the owner on Sep 9, 2020. It is now read-only.

Skip broken vendor symlink rather than returning an error #1190

Closed
wants to merge 1 commit into from
Closed

Skip broken vendor symlink rather than returning an error #1190

wants to merge 1 commit into from

Conversation

wolkykim
Copy link
Contributor

What does this do / why do we need it?

Skip broken vendor symlink rather than returning an error

What should your reviewer look out for in this PR?

When one of 3rd party packages has a broken vendor symlink, it fails whole ensure process.

Current logic behaves:

  • remove a vendor symlink linked to a valid directory
  • leave a vendor symlink linked to a valid file
  • return error if a vendor symlink is broken (linked file doesn't exist)

I think this behavior is not intended and it seems to be that it's better to skip the check except the case it's linked to a valid directory as it's harmless rather than returning an error causing the whole process stop.

Expected/New behavior:

  • remove a vendor symlink linked to a valid directory
  • leave rest of the case(linked to a file or broken) as they are not harmful rather than returning an error

Do you need help or clarification on anything?

no

Which issue(s) does this PR fix?

This is a new behavior introduced in v0.3.1 with its improved type checking
Since this is a breaking change, some users migrating from 0.3.0 could experience this issue.
The main problem is that when this happens on some 3rd party libraries that you don't have the control, you just have to downgrade and use v0.3.0

When one of 3rd pary packages has a broken vendor symlink, it fails whole `ensure` process.
This is a new behavior introduced in v0.3.1 with its improved type checking but I believe
this behavior is not intended and we can skip the check when the symlink appears to be
broken or linked to a file. Only those vendor symlinks that are linked to a directory
need to be removed.
@wolkykim wolkykim requested a review from sdboyer as a code owner September 21, 2017 00:28
@googlebot
Copy link
Collaborator

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.


  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
  • If your company signed a CLA, they designated a Point of Contact who decides which employees are authorized to participate. You may need to contact the Point of Contact for your company and ask to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the project maintainer to go/cla#troubleshoot.
  • In order to pass this check, please resolve this problem and have the pull request author add another comment and the bot will run again.

@wolkykim
Copy link
Contributor Author

I signed it!

@googlebot
Copy link
Collaborator

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for the commit author(s). If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and have the pull request author add another comment and the bot will run again.

@wolkykim wolkykim closed this Sep 21, 2017
@wolkykim wolkykim deleted the remove_broken_vendor_symlink branch September 21, 2017 00:36
@wolkykim wolkykim restored the remove_broken_vendor_symlink branch September 21, 2017 00:36
@wolkykim
Copy link
Contributor Author

wolkykim commented Sep 21, 2017

please ignore this PR and see #1191 instead. Closing

@wolkykim wolkykim deleted the remove_broken_vendor_symlink branch September 21, 2017 01:54
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants