Thanks, Etienne. Comments addressed.
Thank you~
Xintong Song
On Wed, Jul 7, 2021 at 6:41 PM Etienne Chauchot
wrote:
> Hi,
>
> Thanks Xintong for the very good doc ! I added 2 comments to it.
>
> Best,
>
> Etienne
>
> On 02/07/2021 05:57, Xintong Song wrote:
> > Thanks all for the positive
Hi,
Thanks Xintong for the very good doc ! I added 2 comments to it.
Best,
Etienne
On 02/07/2021 05:57, Xintong Song wrote:
Thanks all for the positive feedback.
I have updated the wiki page [1], and will send an announcement in a
separate thread, to draw more committers' attention.
Thanks all for the positive feedback.
I have updated the wiki page [1], and will send an announcement in a
separate thread, to draw more committers' attention.
Moreover, I've opened FLINK-23212 where we can continue with the discussion
around pure documentation changing PRs.
Thank you~
Xintong
I second Tison's opinion.
Similar to how we skip docs_404_check for PRs that do not touch the
documentation, we can skip other stages for PRs that only contain
documentation changes.
In addition to making merging documentation PRs easier, we can also reduce
the workload on CI workers. Especially
I think you are right Tison that docs are a special case and they only
require flink-docs to pass. What I am wondering is how much of a problem
this will be (assuming that we have a decent build stability). The more
exceptions we add, the harder it will be to properly follow the guidelines.
Maybe
Hi,
There are a number of PRs modifying docs only, but we still require all
tests passed on that.
It is a good proposal we avoid merge PR with "unrelated" failure, but can
we improve the case where the contributor only works for docs?
For example, base on the file change set, run doc tests
+1 for the proposal. Thanks Xintong!
Best,
Godfrey
Rui Li 于2021年6月30日周三 上午11:36写道：
> Thanks Xintong. +1 to the proposal.
>
> On Tue, Jun 29, 2021 at 11:05 AM 刘建刚 wrote:
>
> > +1 for the proposal. Since the test time is long and environment may
> vary,
> > unstable tests are really annoying
Thanks Xintong. +1 to the proposal.
On Tue, Jun 29, 2021 at 11:05 AM 刘建刚 wrote:
> +1 for the proposal. Since the test time is long and environment may vary,
> unstable tests are really annoying for developers. The solution is welcome.
>
> Best
> liujiangang
>
> Jingsong Li 于2021年6月29日周二
+1 for the proposal. Since the test time is long and environment may vary,
unstable tests are really annoying for developers. The solution is welcome.
Best
liujiangang
Jingsong Li 于2021年6月29日周二 上午10:31写道：
> +1 Thanks Xintong for the update!
>
> Best,
> Jingsong
>
> On Mon, Jun 28, 2021 at 6:44
+1 Thanks Xintong for the update!
Best,
Jingsong
On Mon, Jun 28, 2021 at 6:44 PM Till Rohrmann wrote:
> +1, thanks for updating the guidelines Xintong!
>
> Cheers,
> Till
>
> On Mon, Jun 28, 2021 at 11:49 AM Yangze Guo wrote:
>
> > +1
> >
> > Thanks Xintong for drafting this doc.
> >
> >
+1, thanks for updating the guidelines Xintong!
Cheers,
Till
On Mon, Jun 28, 2021 at 11:49 AM Yangze Guo wrote:
> +1
>
> Thanks Xintong for drafting this doc.
>
> Best,
> Yangze Guo
>
> On Mon, Jun 28, 2021 at 5:42 PM JING ZHANG wrote:
> >
> > Thanks Xintong for giving detailed documentation.
+1
Thanks Xintong for drafting this doc.
Best,
Yangze Guo
On Mon, Jun 28, 2021 at 5:42 PM JING ZHANG wrote:
>
> Thanks Xintong for giving detailed documentation.
>
> The best practice for handling test failure is very detailed, it's a good
> guidelines document with clear action steps.
>
> +1
Thanks Xintong for giving detailed documentation.
The best practice for handling test failure is very detailed, it's a good
guidelines document with clear action steps.
+1 to Xintong's proposal.
Xintong Song 于2021年6月28日周一 下午4:07写道：
> Thanks all for the discussion.
>
> Based on the opinions so
Thanks all for the discussion.
Based on the opinions so far, I've drafted the new guidelines [1], as a
potential replacement of the original wiki page [2].
Hopefully this draft has covered the most opinions discussed and consensus
made in this discussion thread.
Looking forward to your
Thanks for the clarification Till. +1 for what you have written.
Piotrek
pt., 25 cze 2021 o 16:00 Till Rohrmann napisał(a):
> One quick note for clarification. I don't have anything against builds
> running on your personal Azure account and this is not what I understood
> under "local
One quick note for clarification. I don't have anything against builds
running on your personal Azure account and this is not what I understood
under "local environment". For me "local environment" means that someone
runs the test locally on his machine and then says that the
tests have passed
+1 for the general idea, however I have concerns about a couple of details.
> I would first try to not introduce the exception for local builds.
> It makes it quite hard for others to verify the build and to make sure
that the right things were executed.
I would counter Till's proposal to ignore
+1 for Xintong's proposal.
For me, resolving problems directly (fixing the infrastructure issue,
disabling unstable tests and creating blocker JIRAs to track the fix and
re-enable them asap, etc.) is (in most cases) better than working around
them (verify locally, manually check and judge the
Creating a blocker issue for the manually disabled tests sounds good to me.
Minor: I'm still a bit worried about the commits merged before we fix
the unstable tests can also break those tests. Instead of letting the
assigners keep a look at all potentially related commits, they can
maintain a
I like the idea of creating a blocker issue for a disabled test. This will
force us to resolve it in a timely manner and it won't fall through the
cracks.
Cheers,
Till
On Thu, Jun 24, 2021 at 8:06 AM Jingsong Li wrote:
> +1 to Xintong's proposal
>
> I also have some concerns about unstable
+1 to Xintong's proposal
I also have some concerns about unstable cases.
I think unstable cases can be divided into these types:
- Force majeure: For example, network timeout, sudden environmental
collapse, they are accidental and can always be solved by triggering azure
again. Committers
Thanks to Xintong for bringing up this topic, I'm +1 in general.
However, I think it's still not very clear how we address the unstable
tests.
I think this is a very important part of this new guideline.
According to the discussion above, if some tests are unstable, we can
manually disable it.
Thanks all for the feedback.
@Till @Yangze
I'm also not convinced by the idea of having an exception for local builds.
We need to execute the entire build (or at least the failing stage)
locally, to make sure subsequent test cases prevented by the failure one
are all executed. In that case, it's
Hi Xintong,
+1 to the proposal.
In order to better comply with the rule, it is necessary to describe what's
best practice if encountering test failure which seems unrelated with the
current commits.
How to avoid merging PR with test failures and not blocking code merging
for a long time?
I tried
+1 to Xintong's proposal
On Wed, Jun 23, 2021 at 1:53 PM Till Rohrmann wrote:
> I would first try to not introduce the exception for local builds. It makes
> it quite hard for others to verify the build and to make sure that the
> right things were executed. If we see that this becomes an issue
I would first try to not introduce the exception for local builds. It makes
it quite hard for others to verify the build and to make sure that the
right things were executed. If we see that this becomes an issue then we
can revisit this idea.
Cheers,
Till
On Wed, Jun 23, 2021 at 4:19 AM Yangze
+1 for appending this to community guidelines for merging PRs.
@Till Rohrmann
I agree that with this approach unstable tests will not block other
commit merges. However, it might be hard to prevent merging commits
that are related to those tests and should have been passed them. It's
true that
It is a good principle to run all tests successfully with any change. This
means a lot for project's stability and development. I am big +1 for this
proposal.
Best
liujiangang
Till Rohrmann 于2021年6月22日周二 下午6:36写道：
> One way to address the problem of regularly failing tests that block
>
One way to address the problem of regularly failing tests that block
merging of PRs is to disable the respective tests for the time being. Of
course, the failing test then needs to be fixed. But at least that way we
would not block everyone from making progress.
Cheers,
Till
On Tue, Jun 22, 2021
I think this is overall a good idea. So +1 from my side.
However, I'd like to put a higher priority on infrastructure then, in
particular docker image/artifact caches.
On Tue, Jun 22, 2021 at 11:50 AM Till Rohrmann wrote:
> Thanks for bringing this topic to our attention Xintong. I think your
>
Thanks for bringing this topic to our attention Xintong. I think your
proposal makes a lot of sense and we should follow it. It will give us
confidence that our changes are working and it might be a good incentive to
quickly fix build instabilities. Hence, +1.
Cheers,
Till
On Tue, Jun 22, 2021
Hi everyone,
In the past a couple of weeks, I've observed several times that PRs are
merged without a green light from the CI tests, where failure cases are
considered *unrelated*. This may not always cause problems, but would
increase the chance of breaking our code base. In fact, it has
32 matches
Mail list logo
