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

feat: poll manifest file #69

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

Conversation

kjkao
Copy link

@kjkao kjkao commented Nov 19, 2020

if there are many manifest files in the manifest folder,
it will only poll the specific file.

if there are many manifest files in the manifest folder,
 it will only poll the specific file.
Copy link

@francoisferrand francoisferrand left a comment

Choose a reason for hiding this comment

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

While I understand the idea here, it will not work appropriately when there the manifest include's other manifests : in that case, what the user would expect is that a commit in any of the included manifest will also be found by polling...

As such, I think it would cause more confusion than help, and I would prefer this is not merged.
You could however rework to look at the manifest to list these included manifests, or change the option to make it some include/exclude lists (e.g. poll/ignore some files when polling, similar to git plugin).

(It may be simpler though to rework you job to detect which manifest was modified, and trigger appropriate build accordingly)

commands.add("git");
commands.add("rev-parse");
commands.add("HEAD");
if (pollManifest && manifestFile != null) {

Choose a reason for hiding this comment

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

There is always a manifest file, even if none is specified : it defaults to 'default.xml'

If this poll manifest option is enabled, and the name of manifest is not specified, then it should still check the provided manifest.

@@ -102,6 +102,7 @@
@CheckForNull private boolean resetFirst;
@CheckForNull private boolean cleanFirst;
@CheckForNull private boolean quiet;
@CheckForNull private boolean pollManifest;

Choose a reason for hiding this comment

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

name is misleading, since the manifest is always polled.
maybe pollManifestOnly ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants