Algatux / influxdb-bundle

Bundle service integration of official influxdb/influxdb-php client
MIT License
24 stars 14 forks source link

Form types #57

Closed soullivaneuh closed 8 years ago

soullivaneuh commented 8 years ago

The goal of this PR is to add some useful form types.

The first commit is about the ConnectionRegistry class. This is needed to guess which connection to use.

Those form types can be useful but are totally optional.

This is why the symfony/form package is suggested and not required.

If you have another idea of form type to add, feel free to comment! :+1:

Regards.

Algatux commented 8 years ago

What about metrics (as form type) ?

soullivaneuh commented 8 years ago

What about metrics (as form type) ?

What do you mean? Can you please elaborate?

Algatux commented 8 years ago

A form type with all available metrics stored on the db ? I'm not sure if it's possible. It's the only idea i had :smile:

soullivaneuh commented 8 years ago

You mean the measurements?

Algatux commented 8 years ago

Yeah sorry I was calling them with another name ... some confusion when I wrote. Forget my proposal :cry:

soullivaneuh commented 8 years ago

Another idea (maybe here or on another PR): Add validators.

soullivaneuh commented 8 years ago

@Algatux For me the form types are finished.

I have to add tests and documentation.

Until that, could you please do a first review of the actual codebase?

Algatux commented 8 years ago

@Soullivaneuh I'll do it soon

Algatux commented 8 years ago

it's ok for me

soullivaneuh commented 8 years ago

The Pull Request is now completely finish and ready for my side.

@Algatux: I let you do the final review and merge if everything is OK for you. :+1:

P.S.: Please don't use merge by squash. Those commit are voluntarily separated. :wink:

Regards.

coveralls commented 8 years ago

Coverage Status

Changes Unknown when pulling ed8080bf2ec448f463d63caa724158fbc757c000 on Soullivaneuh:form-types into \ on Algatux:master**.

soullivaneuh commented 8 years ago

@Algatux You should remove the Coveralls PR comments option. It will be post and every pushed commit and the timeline will be quickly unreadable.

soullivaneuh commented 8 years ago

@Algatux If you are ok with https://github.com/Algatux/influxdb-bundle/pull/57#discussion_r73835625, could you please make a new release after this merge? Thanks. :+1:

Algatux commented 8 years ago

@Soullivaneuh I'll take a look asap

soullivaneuh commented 8 years ago

@Algatux great! I also removed @author tags you don't like. Sorry for that, I have the habit to add them..

Algatux commented 8 years ago

@Soullivaneuh review done, it seems ok to me! only please take a look at the two small notes that a left.

soullivaneuh commented 8 years ago

@Algatux OK! Shall I merge it after corrections?

Algatux commented 8 years ago

Sure

soullivaneuh commented 8 years ago

Merged. @Algatux Shall I let you make the next release?

Algatux commented 8 years ago

@Soullivaneuh I think it will be a 2.1.0 ? right ?

soullivaneuh commented 8 years ago

It's a new feature so yes. ;-)

Sullivan SENECHAL

Le 11 août 2016 15:26, "Alessandro Galli" notifications@github.com a écrit :

I think it will be a 2.1.0 ? right ?

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/Algatux/influxdb-bundle/pull/57#issuecomment-239160092, or mute the thread https://github.com/notifications/unsubscribe-auth/ABnqNe0Kr4wVlPiwzid4uwz0DLaIeyqgks5qeyMYgaJpZM4JQzn6 .