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

#23 Support using the library in a non-springboot project #33

Merged
merged 10 commits into from
May 24, 2017
Merged

Conversation

BGehrels
Copy link
Contributor

@BGehrels BGehrels commented May 22, 2017

Splitted the project in three maven modules:

  • one core library (nakadi-producer) without any spring or third party tool dependency (which contains the core algorithms and a set of interfaces which can be implemented by using projects),
  • one spring boot starter (nakadi-producer-spring-boot-starter, which adds autoconfiguration, DB setup and access, etc.) and
  • a reactor POM which just references both.

This will allow to use the core library in non-springboot projects (for #23), but it needs some manual configuration there. We will document this later (in a separate PR) when we have tried it in some internal projects.

It will also allow easier testing of the spring-boot-starter in test projects with different dependency configurations (in later PRs → #56).

BGehrels and others added 5 commits May 22, 2017 15:45
…hout any spring or third party tool dependency, one spring boot starter and one, that ties them all together
…ti module projects, so we remove this. mvnw verify should do the job, and travis by default falls back to ./mvnw install -DskipTests=true -Dmaven.javadoc.skip=true -B -V for the install step which seems fine
@ePaul ePaul self-requested a review May 22, 2017 18:38
…l -DskipTests=true -Dmaven.javadoc.skip=true -B -V for the install step which executed the gpg plugin. this fails, because there is no gpg key. this commit adds -Dgpg.skip=true to it.
@BGehrels
Copy link
Contributor Author

wohoo, build is green \o/

@BGehrels BGehrels mentioned this pull request May 23, 2017
this.eventTransmissionService = eventTransmissionService;
}

public void sendEvents() {
Copy link
Member

Choose a reason for hiding this comment

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

This method previously had a @Scheduled annotation. How is this done now?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Readded it with 5ba13f1

@ePaul
Copy link
Member

ePaul commented May 23, 2017

Should we get a new section into the README like "how to use with non-springboot projects"?

@BGehrels
Copy link
Contributor Author

Would not add this now, maybe later when we have more experiences. Until then, i would consider it undocumented and experimental.

@BGehrels
Copy link
Contributor Author

fixed the comment

@@ -29,6 +29,7 @@
import org.springframework.context.annotation.Configuration;
import org.springframework.context.annotation.Import;
import org.springframework.jdbc.core.namedparam.NamedParameterJdbcTemplate;
import org.springframework.scheduling.annotation.Scheduled;
Copy link
Member

Choose a reason for hiding this comment

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

I guess this import can now go away?

@@ -73,7 +73,7 @@ public void testSnapshotSavedInBatches() {
// when snapshot returns 5 item stream
when(snapshotEventProvider.getSnapshot(PUBLISHER_EVENT_TYPE, null)).thenReturn(eventPayloads.subList(0, 3));
when(snapshotEventProvider.getSnapshot(PUBLISHER_EVENT_TYPE, 2)).thenReturn(eventPayloads.subList(3, 5));
when(snapshotEventProvider.getSnapshot(PUBLISHER_EVENT_TYPE, 5)).thenReturn(Collections.emptyList());
//when(snapshotEventProvider.getSnapshot(PUBLISHER_EVENT_TYPE, 5)).thenReturn(Collections.emptyList());
Copy link
Member

Choose a reason for hiding this comment

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

Why is this not used anymore?

@@ -1,33 +0,0 @@
{"groups": [
Copy link
Member

Choose a reason for hiding this comment

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

This file is gone ... is it not needed anymore?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I do not think it is desperately needed and it i really prone to run out of sync with the code pretty soon, so i deleted it.

Copy link
Member

@ePaul ePaul left a comment

Choose a reason for hiding this comment

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

Looks good.

…it obvious that we had an off-by-one-error here
@BGehrels
Copy link
Contributor Author

👍

1 similar comment
@ePaul
Copy link
Member

ePaul commented May 24, 2017

👍

@ePaul ePaul merged commit 737a6b7 into master May 24, 2017
@ePaul ePaul deleted the fix-docs branch May 24, 2017 17:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants