Skip to content

Conversation

@kbuntrock
Copy link
Contributor

@kbuntrock kbuntrock commented Oct 11, 2023

Description of this pull request

Hello everyone,

A small PR to suggest an evolution : a configuration option to make mandatory the use of the clean phase in order to cache the build result.

The goal is to offer the possibility of denying as much as possible the possibility to cache a faulty build.

What do you think of the idea? :)

EDIT : created the issue in JIRA : https://issues.apache.org/jira/browse/MBUILDCACHE-90

MR checklist

Following this checklist to help us incorporate your
contribution quickly and easily:

  • Make sure there is a MBUILDCACHE JIRA issue filed
    for the change (usually before you start working on it). Trivial changes like typos do not
    require a JIRA issue. Your pull request should address just this issue, without
    pulling in other changes.
  • Each commit in the pull request should have a meaningful subject line and body.
  • Format the pull request title like [MBUILDCACHE-XXX] - Fixes bug in ApproximateQuantiles,
    where you replace MBUILDCACHE-XXX with the appropriate JIRA issue. Best practice
    is to use the JIRA issue title in the pull request title and in the first line of the
    commit message.
  • Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
  • Run mvn clean verify to make sure basic checks pass. A more thorough check will
    be performed on your pull request automatically.
  • You have run the Core IT successfully.

If your pull request is about ~20 lines of code you don't need to sign an
Individual Contributor License Agreement if you are unsure
please ask on the developers list.

To make clear that you license your contribution under
the Apache License Version 2.0, January 2004
you have to acknowledge this by using the following check-box.

NB: Not checked the lines about JIRA since getting rid of it is progress.

@kbuntrock kbuntrock changed the title A "mandatory clean" option to enable the caching functionnality [PR-103] A "mandatory clean" option to enable the caching functionnality Oct 12, 2023
@maximilian-novikov-db
Copy link
Contributor

maximilian-novikov-db commented Oct 19, 2023

@kbuntrock have you hit any real problems after skipping Clean phase?

@kbuntrock
Copy link
Contributor Author

@kbuntrock have you hit any real problems after skipping Clean phase?

Not yet since I'm waiting for my evolution suggestions on outputs + maybe the next official release to adopt it on my projects.

But since going through a clean phase is necessary sometimes to insure nothing from an old build interfere with the current one, I don't want to deal with a clean phase not solving this kind of issues because the faulty build is cached.
Ideally, the use of this extension should be totally transparent for the vast majority of members of my team.

I get that I'm not addressing a daily issue, but I suspect some waste of time from me or another senior dev to investigate the problem the days this occurs.

@olamy olamy added the enhancement New feature or request label Dec 3, 2023
while (it.hasNext()) {
String line = verifier.stripAnsi((String) it.next());
boolean matches = true;
Iterator<String> toMatchIterator = Arrays.stream(texts).iterator();
Copy link
Contributor

@AlexanderAshitkin AlexanderAshitkin Apr 20, 2024

Choose a reason for hiding this comment

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

Not a critical piece of code, but it is more elegant and efficient to use StringUtils#containsAny and Stream#anyMatch:

return Files.lines(filePath) // or lines.stream()
    .anyMatch(line -> StringUtils.containsAny(line, texts));

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. Slightly changed you proposal since the goal is to test the presence of all strings.


@Override
public boolean isMandatoryClean() {
return getConfiguration().isMandatoryClean();
Copy link
Contributor

Choose a reason for hiding this comment

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

It might be handy to add a command line property as well

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done :)

@kbuntrock kbuntrock changed the title [PR-103] A "mandatory clean" option to enable the caching functionnality [MBUILDCACHE-90] A "mandatory clean" option to enable the caching functionnality Apr 24, 2024
@kbuntrock
Copy link
Contributor Author

@olamy: Merge conflicts are resolved. Please merge this one before [MBUILDCACHE-86] (which I will fix after this merge)

@jira-importer
Copy link

Resolve #264

1 similar comment
@jira-importer
Copy link

Resolve #264

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants