[Tarantool-patches] [WIP luajit 00/15] Adapt LuaVela test suites

[Igor Munkin]

Sergos,

Looks like I'm too late here, but I still want to share some thoughts.

On 05.03.21, Sergey Ostanevich wrote:
> Hi!
> 
> 
> > On 4 Mar 2021, at 22:58, Sergey Kaplun <skaplun at tarantool.org> wrote:
> > 
> > On 04.03.21, Sergey Ostanevich wrote:
> >> Hi!
> >> 
> >> Thanks for the patchset - brief review
> >> 
> >> part 1 - obvious, LGTM.
> >> 
> >> part 2 - LGTM, good to have all follow-ups set.
> > 
> > Do you mean to reference all follow-ups inside commit message?
> > 
> 
> There could be just one - if no specific GH created.

Well... In this case the tickets should be mentioned near the changes at
least, but I still think all of them should be referenced in the commit
message. IMHO, it makes the maintenance and software archeology easier.

> 
> >> 
> >> part 3 - should have a follow-up ticket to cover all suppressions, if we plan to fix them.
> > 
> > Ditto.
> > There are some follow ups that can be grepped by qa and luajit labels.
> > 
> 
> And this is something I don’t want to do - to grep using lables. The patch should
> have reference, so I can see all links at once.

Totally agree with you here.

> 
> >> 
> >> part 4-8 are LGTM
> >> 
> >> part 9 (_G and some modules in Tarantool are different): Are we plan to fix it in some way? There should be a follow-up then.
> > 
> > For now I can't see any good solution, except ignoring them by special
> > option like slow tests.
> > 
> 
> I don’t object to ignore them. My question if we plan to tweak the tests to align
> with Tarantool? What will happen to the test if Tarantool will change preloaded
> modules?

It can be tweaked via LUAJIT_TEST_INIT CMake option. I hope most of the
issues will be resolved by it.

> 
> >> 
> >> parts 10-15 are LGTM
> >> 
> >> Regards,
> >> Sergos
> 
> 

-- 
Best regards,
IM