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

Add help for package authors #31

Merged
merged 1 commit into from
May 15, 2017
Merged

Add help for package authors #31

merged 1 commit into from
May 15, 2017

Conversation

MikeInnes
Copy link
Contributor

Making it clear what we require for a merge, and that feedback on names etc is optional.

TAG_REQ = "\n".join((
"Please make sure that:",
"- [ ] CI passes for supported Julia versions (if applicable).",
"- [ ] Version bounds are up to date."
Copy link
Contributor

Choose a reason for hiding this comment

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

this should probably be elaborated on a bit

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, this won't be totally clear to a newbie. Do you have anything we can link to?

Copy link
Contributor

Choose a reason for hiding this comment

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

Not in one place yet, but I should really write up a package doc section on the subject.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That would be a huge boon as far as distributing the workload is concerned, I think. I suspect even many package authors aren't sure exactly what they should be doing here.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'll put that on my to-do list, hopefully right after rc2

@@ -25,6 +25,12 @@

SECRET = os.environ["SECRET"]

TAG_REQ = "\n".join((
"Please make sure that:",
"- [ ] CI passes for supported Julia versions (if applicable).",
Copy link
Contributor

Choose a reason for hiding this comment

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

worth mentioning now that JuliaCIBot will run tests against the package and its reverse dependencies to see if breaking changes are going to cause problems

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looks like this isn't stable yet, right? Once it is we can hopefully just rely on that entirely for package tests.

Copy link
Contributor

Choose a reason for hiding this comment

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

It's getting there pretty quickly. We should ask @nkottary to add a pending status indicator when the job gets triggered so we don't overlook it.

Both are useful to look at. JuliaCIBot isn't going to do well with packages that need custom setup, depend on non-default programs or libraries, etc. Those are usually easier to set up by a package author in their own .travis.yml and are likely non-trivial to replicate completely generally.

Choose a reason for hiding this comment

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

Yes, I can add the pending status indicator for the package tests. Also, I'm going to show the reverse dependency results separately as a comment as that could take an arbitrary amount of time to complete.

Copy link
Contributor

Choose a reason for hiding this comment

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

can we parallelize those across multiple workers?

Choose a reason for hiding this comment

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

It is already running in parallel on a cluster but we currently don't have dynamic scaling capabillity.

Copy link
Contributor

Choose a reason for hiding this comment

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

the pending status is working nicely, thanks!

@simonbyrne
Copy link
Collaborator

I like this. Would you like me to merge it now?

@MikeInnes
Copy link
Contributor Author

Go for it, we can tweak as needed.

@simonbyrne simonbyrne merged commit 3b771bf into attobot:master May 15, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants