Page MenuHomeFreeBSD

Increase WARNS for selected tests
ClosedPublic

Authored by asomers on Mar 9 2017, 8:06 PM.

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

asomers created this revision.Mar 9 2017, 8:06 PM
ngie edited edge metadata.EditedMar 9 2017, 8:22 PM

First off, thank you very much for doing this. It's been on my TODO list for a bit (just further down the stack).

A few questions:

  • have you tested this out with gcc?
  • have you built this on all architectures (I might have a valid "make universe" environment on universe12a... that could be used)?

Also:

  • could you please move this down the WARNS assignment above the first set of .includes?
  • could you please commit this changes and MFC them separately?

Thank you so very much again :):):)!

lib/libpathconv/tests/Makefile
9 ↗(On Diff #26100)

This isn't currently hooked into the build. Please check with Julian about it, since he owns this directory/tests.

In D9933#205387, @ngie wrote:

First off, thank you very much for doing this. It's been on my TODO list for a bit (just further down the stack).

A few questions:

  • have you tested this out with gcc?
  • have you built this on all architectures (I might have a valid "make universe" environment on universe12a... that could be used)?

Not yet; I opened the code review in parallel with build testing. But I'm definitely going to do a full universe on this one.

Also:

  • could you please move this down the WARNS assignment above the first set of .includes?

What?

  • could you please commit this changes and MFC them separately?

Certainly.

Thank you so very much again :):):)!

Adding Julian to review the libpathconv part.

ngie accepted this revision.Mar 9 2017, 10:49 PM
In D9933#205387, @ngie wrote:

First off, thank you very much for doing this. It's been on my TODO list for a bit (just further down the stack).

A few questions:

  • have you tested this out with gcc?
  • have you built this on all architectures (I might have a valid "make universe" environment on universe12a... that could be used)?

Not yet; I opened the code review in parallel with build testing. But I'm definitely going to do a full universe on this one.

Also:

  • could you please move this down the WARNS assignment above the first set of .includes?

What?

Per style.Makefile(5), WARNS should come before CFLAGS/CXXFLAGS (I misremembered that nuance too so I'm guilty of getting it wrong, probably). Not a ship blocker, but if we can nip this in the bud now, that would be awesome :)... (Either way, I'm ok with the change as-is, because it's a net win :)..)!

  • could you please commit this changes and MFC them separately?

Certainly.

Thank you so very much again :):):)!

This revision is now accepted and ready to land.Mar 9 2017, 10:49 PM
julian accepted this revision.Mar 10 2017, 2:21 AM
julian added inline comments.
lib/libpathconv/tests/Makefile
9 ↗(On Diff #26100)

there is a 'change of plans' wih regards to this directory. Bapt has convinced me to add it to libutil instead of having a separate lib, so this will go away. See review D9917

This revision was automatically updated to reflect the committed changes.