Skip to content
This repository has been archived by the owner on Feb 13, 2023. It is now read-only.

Added upload-progress role. #788

Merged
merged 1 commit into from
Dec 10, 2016
Merged

Conversation

thom8
Copy link
Collaborator

@thom8 thom8 commented Jul 9, 2016

@@ -1,6 +1,8 @@
---
- src: arknoll.selenium
version: 1.2.0
- src: beetboxvm.upload-progress
version: master
Copy link
Owner

Choose a reason for hiding this comment

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

Can you add a release? I've learned from past experience that I need to lock in specific versions for stability reasons—especially with things like PHP extensions which can have problems with new OS/PHP version combos.

Also, since this role doesn't currently support RHEL/CentOS, I would rather not include it since there are a large number of users (myself included) who generally use the centos7 box instead of Ubuntu...

@thom8 thom8 force-pushed the upload_progress branch from ea3aa7b to 7fa2063 Compare July 9, 2016 03:22
@thom8
Copy link
Collaborator Author

thom8 commented Jul 9, 2016

@geerlingguy done -- there's no specific Ubuntu tasks so should support all platforms.

@thom8
Copy link
Collaborator Author

thom8 commented Jul 9, 2016

Also no worries if you wanted to maintain this as geerlingguy.upload_progress we'd also switch over to use it as we only want to maintain project roles -- https://github.com/beetboxvm/beetbox/blob/master/provisioning/ansible/config/beetbox.config.yml#L58

@thom8
Copy link
Collaborator Author

thom8 commented Jul 10, 2016

Whilst we don't necessarily want to maintain roles we're more than happy to push PRs upstream and have made it really easy to do this by checking out roles directly from the source repo in debug mode.

https://github.com/beetboxvm/beetbox/blob/master/provisioning/ansible/playbook-roles.yml#L32-L38

@geerlingguy
Copy link
Owner

@thom8 - If you're willing to maintain the role (it seems it would be relatively minimal... but it does take a little effort over time), I'm okay with using it (I use arknoll for selenium :) )—the main thing is if there's ever a bug/problem, I just want to make sure I can help work through it quickly.

If you're not interested in maintaining the role, I'd be willing to fork it and take it over, but only if you don't want it.

@thom8
Copy link
Collaborator Author

thom8 commented Jul 10, 2016

Don't mind either way just thought that you're probably better placed to
support multiple platforms.

It's very similar to your xdebug and xhprof roles.

@thom8
Copy link
Collaborator Author

thom8 commented Jul 12, 2016

I'm happy to maintain it so I'll transfer it to my account and update tests for both platforms.

@thom8 thom8 force-pushed the upload_progress branch from 7fa2063 to 28a97d3 Compare July 12, 2016 10:38
@thom8
Copy link
Collaborator Author

thom8 commented Jul 12, 2016

@geerlingguy what would you recommend as your best role for multi platform tests?

@thom8
Copy link
Collaborator Author

thom8 commented Jul 12, 2016

All done & tests are passing on CentOS 7, Ubuntu 12.04 & 14.04

https://travis-ci.org/thom8/ansible-role-php-upload-progress

@thom8 thom8 force-pushed the upload_progress branch from 28a97d3 to 2bf3409 Compare July 12, 2016 13:01
@geerlingguy
Copy link
Owner

It looks like 1.0.1 isn't available on Galaxy yet:

[WARNING]: - thom8.php-upload-progress was NOT installed successfully: ERROR!
- the specified version (1.0.1) of thom8.php-upload-progress was not found in
the list of available versions ([{u'name': u'1.0.0', u'created':
u'2016-07-12T10:35:16.650Z', u'url': u'', u'release_date':
u'2016-07-09T07:20:59Z', u'modified': u'2016-07-12T10:35:17.005Z', u'related':
{}, u'id': 16514, u'active': True, u'summary_fields': {u'role': {u'id': 10928,
u'name': u'php-upload-progress'}}}]).

https://galaxy.ansible.com/thom8/php-upload-progress/

Note that you can have Travis push updated tags directly to Galaxy if you get your travis token from your Travis CI settings page (on left side near top), then go to Galaxy, 'My Roles', then click on the settings widget for your role and add in the token.

Then you add the notifications config to .travis.yml (see one of my roles for copy-paste example), and Travis will push tags automatically.

@thom8 thom8 force-pushed the upload_progress branch from 2bf3409 to 0002ba1 Compare July 12, 2016 14:55
@thom8
Copy link
Collaborator Author

thom8 commented Jul 12, 2016

Thanks -- rebuilding now

@thom8
Copy link
Collaborator Author

thom8 commented Jul 19, 2016

@geerlingguy - anything outstanding on this one?

@geerlingguy geerlingguy added this to the 3.3.0 milestone Jul 27, 2016
@geerlingguy geerlingguy modified the milestones: 3.4.0, 3.3.0 Oct 1, 2016
@geerlingguy geerlingguy modified the milestones: 3.6.0, 3.4.0 Nov 16, 2016
@geerlingguy
Copy link
Owner

I promise I'll get back to this soon. Sorry for the delays in review!

@geerlingguy
Copy link
Owner

I promise I'll get back to this soon. Sorry for the delays in review!

Says the guy who keeps putting it off... I'm working on a 3.6.0 release very soon—any chance you can fix the conflicts, @thom8 ?

@thom8 thom8 force-pushed the upload_progress branch 3 times, most recently from 2bcf3c2 to 442eb27 Compare December 10, 2016 00:35
@thom8
Copy link
Collaborator Author

thom8 commented Dec 10, 2016

@geerlingguy done -- also changed to be conditionally installed and included v1.0.1 of the role in the PR :)

@geerlingguy geerlingguy merged commit 75ad0a2 into geerlingguy:master Dec 10, 2016
@geerlingguy
Copy link
Owner

Beautiful! Thanks for your work on this. It may become a default at some point... but maybe not, since it's one of those things that people don't often add in (if they don't get it OOTB) unless they actually need it.

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

Successfully merging this pull request may close these issues.

2 participants