Conversation
| */ | ||
| package com.cognifide.qa.bb.email; | ||
|
|
||
| public interface EmailClientFactory { |
| import com.google.inject.assistedinject.AssistedInject; | ||
| import com.google.inject.name.Names; | ||
|
|
||
| public class EmailConfig { |
There was a problem hiding this comment.
Missing javadocs on all public methods and the class itself - and same applies to other classes in this PR.
| */ | ||
| package com.cognifide.qa.bb.email.helpers; | ||
|
|
||
| public interface EmailDataGeneratorFactory { |
| new FactoryModuleBuilder().build(MailServerFactory.class), | ||
| new FactoryModuleBuilder().build(EmailDataGeneratorFactory.class) | ||
| ); | ||
| injector.injectMembers(this); |
There was a problem hiding this comment.
Are we still injecting any members?
| <!-- tests --> | ||
| <dependency> | ||
| <groupId>org.mockito</groupId> | ||
| <artifactId>mockito-core</artifactId> |
There was a problem hiding this comment.
The scope for this dependency is inherited from parent pom. Please remove it from here.
(should have)
|
|
||
| private static final Logger LOGGER = LoggerFactory.getLogger(EmailDataFactory.class); | ||
|
|
||
| private final Pattern addressPattern; |
There was a problem hiding this comment.
Why is final keyword removed here?
For me it looks like a hint for developer that this is not mutable (and needs to be set in constructor).
Could we have it restored to final ?
(should have).
| @@ -1,12 +0,0 @@ | |||
| email.username=qatest_user | |||
There was a problem hiding this comment.
When managing multiple mailboxes it would be easier for me to have separate properties file for every mailbox.
Then if keys didn't contain ${id} (id could be a filename without extension) then i could easily compare those files (when for example the difference should be only in username/password.
- less changes in this pull request
- configuration id easy to change (one change instead of change in every key)
- creating a new configuration easy (just copy a file)
What do you think?
wiiitek
changed the title
3 participants
No description provided.