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

implement Match module functions #116

Merged
merged 1 commit into from
Sep 23, 2015
Merged

implement Match module functions #116

merged 1 commit into from
Sep 23, 2015

Conversation

lucidd
Copy link
Member

@lucidd lucidd commented Sep 22, 2015

This implements some of the functions in the match Module

@lucidd lucidd force-pushed the match-module branch 3 times, most recently from 01a14f0 to 65585c0 Compare September 22, 2015 13:48
*/
public static String join(List<String> list, String delimiter) {
if (list.isEmpty()) {
return "";
Copy link

Choose a reason for hiding this comment

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

42!

This is not nice.
Especially, if you're fun of lambdas. 😉

    public static String join(List<String> list, String delimiter) {
        StringBuilder out = new StringBuilder();
        if (list != null && !list.isEmpty()) {
            out.append(list.get(0));
            list.subList(1, list.size()).stream().forEach((item) -> {
                out.append(delimiter).append(item);
            });
        }

        return out.toString();
    }

/**
* salt.modules.match
*/
public class Match {
Copy link

Choose a reason for hiding this comment

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

BTW, @renner suggests to have at least have a documentation for the whole class with the URL link to it. Maybe Johannes will write more here.

Copy link
Member Author

Choose a reason for hiding this comment

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

I will open a separate issue to track the documentation of the modules where we also can discuss what to do. In any case adding documentation should be done in a separate PR that covers all modules.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for opening the issue, @lucidd! From my perspective there is no need to add javadoc for the Match module class within this patch, especially I wouldn't want to copy it from the salt documentation for the reasons mentioned in #117. For now let's just do it as the other module classes we already have, i.e. keep it as it is.

@@ -63,5 +63,4 @@ public void testStreamToString() {
new ByteArrayInputStream(TEST_STRING.getBytes()));
assertEquals("Result doesn't match test string", TEST_STRING, result);
}

Copy link
Member

Choose a reason for hiding this comment

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

While this change makes sense it does not actually seem to belong to this patch.

Copy link
Member Author

Choose a reason for hiding this comment

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

Oops seems like a left over from a previous version i will remove it. (or rather add it back 😄 )

@lucidd lucidd force-pushed the match-module branch 2 times, most recently from 4113290 to 158b8e6 Compare September 23, 2015 08:58
@renner renner added this to the Version 0.6.0 milestone Sep 23, 2015
lucidd added a commit that referenced this pull request Sep 23, 2015
implement Match module functions
@lucidd lucidd merged commit a336965 into master Sep 23, 2015
@lucidd lucidd deleted the match-module branch September 23, 2015 12:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants