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 DataGridExtensionFunctionalTest #983

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

plfort
Copy link
Contributor

@plfort plfort commented Nov 16, 2017

No description provided.

@plfort
Copy link
Contributor Author

plfort commented Nov 17, 2017

My bad, I didn't test on symfony 2.* branches. I will fix it and the default locale problem

@plfort
Copy link
Contributor Author

plfort commented Nov 17, 2017

@APY/collaborators are you ok with that ?

Copy link
Member

@DonCallisto DonCallisto left a comment

Choose a reason for hiding this comment

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

I will not provide a technical review as I literally don't know what @hmert is doing: he's one of the maintainer but he'll keep to be uncommunicative with all contributors.

My impression is that every PR is accepted and this bundle, due to this fact, could become quickly a huge mess.

I would like to have a major commitment of everyone on this project otherwise this is going to die imho.

@@ -0,0 +1,27 @@
<?php
/**
Copy link
Member

Choose a reason for hiding this comment

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

Remove this please

{

/**
* You can modify the container here before it is dumped to PHP code.
Copy link
Member

Choose a reason for hiding this comment

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

As well.

);
}

/**
Copy link
Member

Choose a reason for hiding this comment

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

Remove this

}


/**
Copy link
Member

Choose a reason for hiding this comment

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

Remove this

use Symfony\Component\Routing\RouterInterface;

/**
* Class DataGridExtensionFunctionalTest.
Copy link
Member

Choose a reason for hiding this comment

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

Please remove this.

@DonCallisto
Copy link
Member

@plfort maybe is better to squash everything in one commit?

Drop travis tests on 2.7 because of "shared" attribute in service container definitions

Fix test AppKernel for symfony 2.8

Set default locale to en-US

Remove useless comment/use statements
Format code
@plfort plfort force-pushed the test-improvement-twigextension branch from b09de60 to 46433ba Compare November 19, 2017 13:29
@plfort
Copy link
Contributor Author

plfort commented Nov 19, 2017

@DonCallisto is it ok for you ?

@DonCallisto
Copy link
Member

As I told, it's fine conceptually; I did not check it technically as I'm still waiting for an answer from @hmert

@hmert
Copy link
Contributor

hmert commented Nov 22, 2017

@DonCallisto Hi,
I've our old setup on my local env based on academic/vipa . I've tested your tests with data each PR on it. So basically if nothing is broken, I'm willing to approve the PR not merge. If I've approved, this is my contribution. If anyone or I merged PR, it affects everyone.

My impression is that every PR is accepted and this bundle, due to this fact, could become quickly a huge mess.

I don't agree. As you know, 2.x was returned to life from last years contributions. At that point, those who opened PR's has issues but want to use this bundle. We should help them because except few, everyone has lost enthusiasm to use/contribute. The owner has tried to answering questions. Like us.

I would like to have a major commitment of everyone on this project otherwise this is going to die imho.

Our team was not full time dedicated to maintain this project, like you. So a limited time everyone should have a commitment. On the other hand this project will die or not, being maintained does not mean to be died. It will/should evolve. I want to thank all contributors. But this project completely should change its infrastructure. A complete rewrite progress is long and needs commitment.

The issue was that we are not a community, yet.

@DonCallisto
Copy link
Member

Hi @hmert
Sadly, I couldn't disagree more with you.

Starting with

I've tested your tests with data each PR on it. So basically if nothing is broken, I'm willing to approve the PR not merge.

First of all, approving a PR is not about testing, it's about code review. Second, the minimum test coverage is not reached yet so it's virtually impossible to say if something is good or not without looking the code and trusting only travis. I've never seen a coding standard applied, never seen a rejection or a discussion for one improvement. Never.

I don't agree. As you know, 2.x was returned to life from last years contributions. At that point, those who opened PR's has issues but want to use this bundle. We should help them because except few, everyone has lost enthusiasm to use/contribute.

I don't disagree with this. I strongly disagree with this. Since I'm a contributor of this bundle, you and the others disappear from there. No one tag an issue, no one reply to an issue or close it as invalid, no one coordinate, no one answer (I've tagged you and other collaborators more than once and never got a reply). Please, take a look to issues, PR(s) and others since one year: I'm the only one, along with @AlessandroMinoccheri that helped me with tests, that has done an active work on this bundle. You didn't.

The owner has tried to answering questions. Like us.

That's not a point at all, sorry. New owners should be more active than previous one, otherwise no improvements will be reached.

Our team was not full time dedicated to maintain this project, like you.

I don't dedicate my full time on this, but my free time. You're not full time on this, neither free time: in last year you were not on the project at all.

On the other hand this project will die or not, being maintained does not mean to be died. It will/should evolve.

Yes it will die as soon as people starts to abandon it. It dies if someone would like to use it but see that this is a one developer bundle, see that there's a lot of issues, there's not a commitment by a community; he/she will never start using this bundle.

The issue was that we are not a community, yet.

That's the only point I agree. If you really believe that we need to become a community, you and all collaborators should improve their efforts.
I've wrote you in July to ask for news, you replied me to wait few days for an extended reply: you never replied till today.
I'm not willing to waste time in this project if I'm the only one, because basically, at least at my eyes, tomorrow this project could become abandoned: I prefer to spend my free time in something that is or will be used, something where there's a commitment, not where it pretend to be. For months I've asked for help with test, to assign one or two to each contributor but no one replied; the difference between have or be not willing to spend time in this and no reply at all is huge.

So, if your commitment in this is limited to what you have done in last year, please, try to find new maintainers.

@hmert
Copy link
Contributor

hmert commented Nov 24, 2017

@DonCallisto I'm here to make something better as you, not better then you. I do my best so far. You could blame it all on me, It's OK. If it will help, I could step down, now.

@DonCallisto
Copy link
Member

DonCallisto commented Nov 24, 2017

@hmert I'm not here to claim that you should do better than me, or dedicate more time than me. I'm not here to say that I've done more than you. I'm just here to say that you, as the maintainer of this bundle, should be more present than "approving PR".
If you're not ready for this, please, let's search together a new maintainer. I suppose it's the best for the bundle.
Otherwise, please increase your presence: try to involve all @APY/collaborators, let's coordinate everyone, let's discuss more when someone open an issue (maybe tag them) and so on: you know, everything that a maintainer should do.

Don't be mad at me, I'm not accusing you; I do know that everyone is involved with its time but to be "vanish" for an entire year without answering to tags in issue and this kind of things is a huge signal that something is not working.

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.

3 participants