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

Don't deploy documentation on cron jobs #917

Merged
merged 2 commits into from
Dec 28, 2018
Merged

Don't deploy documentation on cron jobs #917

merged 2 commits into from
Dec 28, 2018

Conversation

christopher-dG
Copy link
Contributor

It's not really useful to deploy the same commit repeatedly.
Docs on the env variable can be found here (TRAVIS_EVENT_TYPE).

Copy link
Member

@fredrikekre fredrikekre left a comment

Choose a reason for hiding this comment

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

There is some logic that skips deployment when nothing has changed, but I have noticed there are always some small changes anyway. For example 68fe03b was built for the same Documenter commit as the previous one. This PR seem OK to me. Would be good to document all of the criteria in the manual, but does not need to be in this PR.

@mortenpi
Copy link
Member

LGTM, makes sense to have this. Would you mind adding a note about this to the CHANGELOG under a v0.22-dev heading?

Ideally, we would have the output be more deterministic. I think the sorting of methods / docstrings is a bit stochastic sometimes?

@mortenpi mortenpi added this to the 0.22 milestone Dec 22, 2018
@mortenpi mortenpi merged commit de3d4d3 into JuliaDocs:master Dec 28, 2018
@mortenpi
Copy link
Member

Thanks!

@dehann
Copy link

dehann commented Apr 7, 2019

Hi, could you please point me to the right approach on how to deploy docs from Travis? Since v0.22.x it looks like the "cron" on master branch for deployment is no longer a viable way to do it:

┌ Info: Deployment criteria:-ENV["TRAVIS_REPO_SLUG"]="JuliaRobotics/Caesar.jl" occurs in repo="github.com/JuliaRobotics/Caesar.jl.git"-ENV["TRAVIS_PULL_REQUEST"]="false" is "false"-ENV["TRAVIS_TAG"]="" is (i) empty or (ii) a valid VersionNumber
│ -ENV["TRAVIS_BRANCH"]="master" matches devbranch="master" (if tag is empty)
│ -ENV["DOCUMENTER_KEY"] exists
│ -ENV["TRAVIS_EVENT_TYPE"]="cron" is not "cron"
└ Deploying: ✘
The command "julia --project=docs/ docs/make.jl" exited with 0.

I have looked at Documenter.jl/.travis.yml and the docs, but do not see where else you would set it up?

Reference:

Thanks in advance!

@christopher-dG
Copy link
Contributor Author

I took a look at your Travis log and it's really confusing.. It looks like your jobs are erroneously being marked as cron (i.e. a Travis bug). Maybe send their support an email to inquire.

@christopher-dG
Copy link
Contributor Author

Unless you indeed have some weird cron setup that runs only once on PR merge. Do you have any Travis settings tweaked from the defaults?

@dehann
Copy link

dehann commented Apr 8, 2019

Thanks for taking a look.

It looks like your jobs are erroneously being marked as cron (i.e. a Travis bug).

The log is standard Travis cron job running once a week. I would expect ENV["TRAVIS_EVENT_TYPE"]="cron" to be correct behavior. How else should docs be deploying, if not on cron builds?

Unless you indeed have some weird cron setup that runs only once on PR merge.

FYI, I am not running tests on master commit, and maybe that is why? My thinking a while back was test on PR and weekly "cron" builds only, thereby reducing Travis load.

Do you have any Travis settings tweaked from the defaults?

I don't think so. "Build pushed branches" is turned OFF. "Build pushed pull requests" is ON.

@dehann
Copy link

dehann commented Apr 8, 2019

Will quickly try with "Build pushed branches" ON.

@mortenpi
Copy link
Member

mortenpi commented Apr 8, 2019

Yep, the deployment is meant to happen on "push to master" builds. However, if you do need deployment on cron builds, I'd happily take a PR that would allow for this to be configured in deploydocs.

@christopher-dG
Copy link
Contributor Author

christopher-dG commented Apr 8, 2019

In general, you should definitely be building on pushes to master (here is some justification)).

If you want to keep your existing CI setup, you can also manually set the environment variable TRAVIS_EVENT_TYPE to be something other than "cron" by updating the julia command in your documentation stage to be something like TRAVIS_EVENT_TYPE="" julia -e ....

Edit: It would probably be better to instead update make.jl to include something like:

get(ENV, "TRAVIS_EVENT_TYPE", "") == "cron" && delete!(ENV, "TRAVIS_EVENT_TYPE")

@dehann
Copy link

dehann commented Apr 8, 2019

That was it, thanks!

┌ Info: Deployment criteria:-ENV["TRAVIS_REPO_SLUG"]="JuliaRobotics/Caesar.jl" occurs in repo="github.com/JuliaRobotics/Caesar.jl.git"-ENV["TRAVIS_PULL_REQUEST"]="false" is "false"-ENV["TRAVIS_TAG"]="" is (i) empty or (ii) a valid VersionNumber
│ -ENV["TRAVIS_BRANCH"]="master" matches devbranch="master" (if tag is empty)
│ -ENV["DOCUMENTER_KEY"] exists
│ -ENV["TRAVIS_EVENT_TYPE"]="push" is not "cron"
└ Deploying: ✔
Initialized empty Git repository in /tmp/tmp50WrnT/.git/
...

Think I initially went down the wrong path due to

-ENV["TRAVIS_EVENT_TYPE"]="cron" is not "cron"

rather than something like ENV["TRAVIS_EVENT_TYPE"]="cron", will only deploy on "push" (master). Either way not serious, was still able to find it.

Thanks again for the help!

@christopher-dG
Copy link
Contributor Author

I can definitely see how the notation might be confusing. I wonder if changing the order would be clearer:

- ENV["TRAVIS_EVENT_TYPE"]="cron" is not "cron":  ✘

@dehann
Copy link

dehann commented Apr 11, 2019

In my case, I don't think swapping the order would have worked for me. I started searching for issues and hadn't found one yet -- hope this thread helps in the future.

I looked at the code a little and didn't want to add too many if statements for just that one line. That would change the behavior for different cases of "is not cron" in different cases. I think this issue discussion should be enough. If this happens again and more people comment for "cron is not cron" to be little clearer, then perhaps worth it...

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

Successfully merging this pull request may close these issues.

4 participants