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

Configure AppVeyor to ignore non-source commits #1224

Closed
MV10 opened this issue Oct 1, 2018 · 9 comments
Closed

Configure AppVeyor to ignore non-source commits #1224

MV10 opened this issue Oct 1, 2018 · 9 comments
Labels

Comments

@MV10
Copy link

MV10 commented Oct 1, 2018

Open for discussion before I start submitting PRs:

In all the repos I've checked, AppVeyor builds and deploys a new package for every commit, regardless of scope or content. It is possible to restrict the configuration to only build when source or other build-related files are changed, such that changes to non-package content like README files or unit tests will not trigger a new package.

I intend for this to be a tracking issue across many Serilog repos (just as Maxime did for #1219 to update many SLNs at once).

Here is an example (the assets folder is included because it's home to the Serilog strong-name signing key and Build.ps1 is the AppVeyor script):

only_commits:
  files:
    - serilog-settings-configuration.sln
    - src/Serilog.Settings.Configuration/
    - Build.ps1
    - assets/

Since Maxime already did the hard work of tracking down which repos are in active development, I've followed his lead and will update mostly the same list. I've already forked and branched all of them and by the end of today I should have all the configs updated and ready to PR.

Please chime in if you see something I've overlooked, or if you're active in any of these repos and know of special handling they might require. (I realize not all repos follow exactly the same src/sln naming pattern, I'll handle that correctly on a case-by-case basis.) A case I'm wondering about is whether any repo uses AppVeyor for PR unit test validation. The repos I've been involved with all have a separate TravisCI process that covers this regardless of whether AppVeyor runs.

This was referenced Oct 1, 2018
@merbla
Copy link
Contributor

merbla commented Oct 7, 2018

@MV10 could we include tests? I would prefer a build runs on any change to tests.

@MV10
Copy link
Author

MV10 commented Oct 8, 2018

@merbla Are you referring to this repo, or in general? Serilog has Travis and that includes tests without uploading a new NuGet dev package.

It would be just one more line to reference the test directory but I figured even in repos without Travis you're unlikely to only commit tests without corresponding app code changes.

@merbla
Copy link
Contributor

merbla commented Oct 8, 2018

My thoughts were in general for all the repositories in the Serilog organisation. In the case of the Serilog core repository, we only get a release version when we merge to master, so incremental changes to minor files won't really cause too much churn in terms of NuGet release packages. This is the case for the majority of sinks (although some do have their differences).

AppVeyor is the default build system ATM for the Serilog org. TravisCI was established back when .Net Core was first being established and builds were flaky across OS platforms.

Perhaps TravisCI could even be deprecated now that AppVeyor has more features re: cross platform.

e.g.

image:
  - Visual Studio 2017
  - Ubuntu

@MV10
Copy link
Author

MV10 commented Oct 8, 2018

@merbla Ok, I'll get on it later today. I kind of wondered about the Travis vs AppVeyor thing, they did seem to cover almost the same ground. I'd be happy to see Travis go away, it's painfully slow and I hate walking away from a PR not having full confirmation that something didn't go sideways. I think out of the list I updated only four repos use it.

And you're right, for exactly the same reasons I voiced (test commits without app changes will be rare) this change would produce minimal churn while retaining test validations.

@MV10
Copy link
Author

MV10 commented Oct 8, 2018

All PRs updated (well, all of them which have unit tests, a few do not).

If anyone wants to pursue getting rid of Travis, these use it out of the list I worked with:

  • serilog
  • serilog-sinks-console
  • serilog-sinks-debug
  • serilog-sinks-file
  • serilog-sinks-splunk
  • serilog-settings-configuration

@tsimbalar
Copy link
Member

I'm probably coming here late, but I think the main thing we want to avoid is generating new builds when tweaking documentation. So ... wouldn't it be enough to not build when the changes are only modifications to files like **/*.md, LICENSE ?

From what I read in the docs, that would mean using skip_commits ...

Using a black-list instead of a white-list also means that if we add new projects in the repo, there is no need to remember to tweak the appveyor file... and it also makes it more copy-pasteable across repos.

@MV10 I know you've already invested time in those massive modifications, so this is mostly a comment, open for discussion :)

@merbla
Copy link
Contributor

merbla commented Oct 9, 2018

+1 for a black-list approach. I think allows more flexibility for each sink.

Sorry @MV10 for the churn.

@nblumhardt
Copy link
Member

Guessing @MV10 has a script for updating those en-masse, so may not be much of a change... Any thoughts on blacklisting instead, @MV10 ?

@MV10
Copy link
Author

MV10 commented Oct 13, 2018

Unfortunately the repos aren't consistent enough to script it (whitelist or blacklist), but I can see the case for a blacklist approach. Probably won't have time for this until next weekend, unfortunately.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants