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

make loomio X work #1781

Merged
merged 1 commit into from Feb 27, 2015
Merged

make loomio X work #1781

merged 1 commit into from Feb 27, 2015

Conversation

rdbartlett
Copy link
Contributor

@rob how's this?

here's what I did

  • redirect / to /sign_in rather than marketing index
  • change all references from 'Loomio' to hostname
  • new footer:
    "{{hostname}} is running an independent copy of Loomio - find out more"
  • new in-app logo

@gdpelican
Copy link
Contributor

@rdbartlett is this something you want to keep around?

@denjello
Copy link
Contributor

denjello commented Feb 6, 2015

Definitely - this pull request is SUPER important. I ended up doing this all by hand for our "independent copy". That and deleted the loomio.org google tracking system...

@gdpelican
Copy link
Contributor

It if it works (or almost works) we should spruce it up and merge it in, otherwise it's going to end up too stale to use.

@robguthrie
Copy link
Member

Sprucing this up would be amazing. We really want it.. even if it's not perfect (visually), we'd rather merge it and improve than lose it.

@gdpelican
Copy link
Contributor

I squished it and (I think) made the failing test pass. Be cool to get this merged in if @rdbartlett thinks it's okay to go. 🍊

@rdbartlett
Copy link
Contributor Author

I think it is ok but that is not exactly a strong QA. Have you smoke tested this @gdpelican?

@gdpelican
Copy link
Contributor

Very much no, and it looks like, while the specs pass, there are a few cukes still failing. Guess I'll leave this open for now, but it'd be a huge help to have someone take a look at some point soonish.

@mixmix
Copy link
Contributor

mixmix commented Feb 20, 2015

ima look at those tests now

@rdbartlett
Copy link
Contributor Author

Thanks Mix. Happy to attempt to answer questions. Haven't managed to
prioritised this work myself
On 21/02/2015 12:02 PM, "mix irving" notifications@github.com wrote:

ima look at those tests now


Reply to this email directly or view it on GitHub
#1781 (comment).

@mixmix
Copy link
Contributor

mixmix commented Feb 20, 2015

hey are y'all using spring ?
e.g.

bin/cucumber features/contact_form.feature:3 
bin/spring status

noting the first run is normal speed while spring pre-loads rails test env

@mixmix
Copy link
Contributor

mixmix commented Feb 20, 2015

test travis state local state
features/contact_form.feature:3
features/users/beta_logo.feature:13
features/users/reset_password.feature:10

I tested both the specified failing test (by line) and the whole feature file in each case.
confirms legit failures

- else
%footer.footer
.col-xs-12
= t :"footer.independent_install_html", hostname: ENV['CANONICAL_HOST']
Copy link
Contributor

Choose a reason for hiding this comment

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

I think that ENV reference should ideally be ideally abstracted / wrapped in a method like :

= t :"footer.independent_install_html", hostname: canonical_hostname

so we can define some fallbacks somewhere and have options about editing easily in future

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah I agree. ENV in the view is bad form.

@mixmix
Copy link
Contributor

mixmix commented Feb 21, 2015

should be all green now.
then on to some solid QA

we need to test subscription user + generic user + custom installer

@robguthrie
Copy link
Member

How can I know when this is ready to go?

@mixmix
Copy link
Contributor

mixmix commented Feb 25, 2015

I don't know if anyone is holding this.
I just took it to green.

It needs someone with there had around what it supposed to do to check is
doing what it should.
I modified the tests and tried to do each change in a different commit.

I can pair with someone on Friday if i have time,
Otherwise recommend we ask rich or one of the shoes picks this up to get it
merged
On 25/02/2015 2:23 pm, "Robert Guthrie" notifications@github.com wrote:

How can I know when this is ready to go?


Reply to this email directly or view it on GitHub
#1781 (comment).

@gdpelican
Copy link
Contributor

I think @rdbartlett wants to make a call here, either by providing a QA, or a list of things that should be checked before we merge this.

@robguthrie
Copy link
Member

Mix I'll take you up on pairing on friday.

On Thu, Feb 26, 2015 at 12:14 AM, James Kiesel notifications@github.com
wrote:

I think @rdbartlett https://github.com/rdbartlett wants to make a call
here, either by providing a QA, or a checklist of things that should be
checked before we merge this.


Reply to this email directly or view it on GitHub
#1781 (comment).

@mixmix
Copy link
Contributor

mixmix commented Feb 25, 2015

Would love a check list to guide.

Rob I've cal invited you. Only other times that work for me would involve
swapping things around, which might work, but more complicated.
Looking forward to smashing some code with you!
On 26/02/2015 6:14 am, "Robert Guthrie" notifications@github.com wrote:

Mix I'll take you up on pairing on friday.

On Thu, Feb 26, 2015 at 12:14 AM, James Kiesel notifications@github.com
wrote:

I think @rdbartlett https://github.com/rdbartlett wants to make a call
here, either by providing a QA, or a checklist of things that should be
checked before we merge this.


Reply to this email directly or view it on GitHub
#1781 (comment).


Reply to this email directly or view it on GitHub
#1781 (comment).

@rdbartlett
Copy link
Contributor Author

Thanks for picking this up dudes. The checklist is the first post in this thread.

Conflicts:
	config/locales/en.yml
@mixmix
Copy link
Contributor

mixmix commented Feb 27, 2015

this has been QA'd by rob and I
squashed and ready to go

@mixmix
Copy link
Contributor

mixmix commented Feb 27, 2015

"we should consider how to unobtrusively track third party loomio installs" - Rob

@gdpelican
Copy link
Contributor

So cool. ✳️

@gdpelican
Copy link
Contributor

Cool, thanks so much guys. Feels fuzzy good to merge this. 🍍

gdpelican added a commit that referenced this pull request Feb 27, 2015
@gdpelican gdpelican merged commit 3033e22 into master Feb 27, 2015
@gdpelican gdpelican deleted the loomioX branch February 27, 2015 10:57
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.

None yet

5 participants