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

skip xamarin-dependent steps in appveyor PR builds #1061

Merged
merged 2 commits into from
Nov 25, 2015
Merged

skip xamarin-dependent steps in appveyor PR builds #1061

merged 2 commits into from
Nov 25, 2015

Conversation

nathan-schubkegel
Copy link
Contributor

Experimenting based on the discussion started at #1044 (comment)

…pt secure variable xamarin_password, so appveyor builds no longer fail for pull requests
@nathan-schubkegel
Copy link
Contributor Author

This is the workaround suggested by Oystein Bjorke at http://help.appveyor.com/discussions/questions/1728-register-xamarin-license-for-pull-requests using windows command line "IF DEFINED" to skip build steps when it's obvious they're going to fail.

IF DEFINED xamarin_password appveyor RegisterXamarinLicense ...

The theory is based on http://www.appveyor.com/docs/lang/xamarin where it says

Secure variables are not decrypted during pull request (GitHub only) 
builds of public projects. This is made intentionally to avoid leaking your 
credentials by submitting PR with malicious build script displaying 
environment variables.

@codecov-io
Copy link

Current coverage is 73.05%

Merging #1061 into master will not affect coverage as of 6d8b35e

@@            master   #1061   diff @@
======================================
  Files          263     263       
  Stmts        14897   14897       
  Branches      1623    1623       
  Methods          0       0       
======================================
  Hit          10883   10883       
  Partial        418     418       
  Missed        3596    3596       

Review entire Coverage Diff as of 6d8b35e


Uncovered Suggestions

  1. +0.09% via ...nternal/AspHelper.cs#206...218
  2. +0.09% via ...nternal/AspHelper.cs#188...200
  3. +0.09% via ...nternal/AspHelper.cs#170...182
  4. See 7 more...

Powered by Codecov. Updated on successful CI builds.

@304NotModified 304NotModified self-assigned this Nov 25, 2015
@304NotModified
Copy link
Member

thanks! But it seems that it's now never build on Xamarin., See https://ci.appveyor.com/project/nlog/nlog/build/4.0.1278

I expect:

NLog.Xamarin.Android -> C:\projects\nlog\build\bin\Debug\Xamarin.Android\NLog.dll

@nathan-schubkegel
Copy link
Contributor Author

Yeah. A private CI build (meaning "Wherever the AppVeyor builds work fully for you before these changes") will still run the xamarin stuff because %xamarin_password% expands correctly there. So for git pull request workflow, maybe the idea is at some point, someone does a private CI build to check the Xamarin stuff before accepting a pull request (bleh, more work). Or we just let it go and retroactively fix stuff on 'master' after the fact (ew).

My most recent commit attempted to produce a warning that would make it obvious in the GitHub web display when xamarin steps are skipped (but it doesn't look like it made a noticeable difference like I wanted)

This all sounds like more steps forward than back to me (because at least what can be built is building). But if it's not good enough, I'm OK to cancel this pull request and let someone else find a better solution =)

@304NotModified
Copy link
Member

I will merge this as it's better than nothing....

Thanks for your help! I will contact AppVeyor as I think to know a solution for the secure variables "leak" in PR's

304NotModified added a commit that referenced this pull request Nov 25, 2015
…rin-builds-for-pull-requests

skip xamarin-dependent steps in appveyor PR builds
@304NotModified 304NotModified merged commit feb2592 into NLog:master Nov 25, 2015
@304NotModified 304NotModified added the enhancement Improvement on existing feature label Nov 25, 2015
@304NotModified 304NotModified added this to the 4.3 milestone Nov 25, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improvement on existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants