Hi Stefano, thanks for the feedback.. Just some minor comments inline
2010/12/15 Stefano Bagnara <apa...@bago.org>: > 2010/12/14 Norman Maurer <nor...@apache.org>: >> Hi there, >> >> after spending some thoughts about last optimization before the next >> release, I stumpled again over the MailServer interface. >> The Mailserver interface has corrently 5 methods listed. I think a few >> of them should be moved and a few should get removed at all. Let me >> propose the following changes: >> >> * MailServer.getId(); >> This method is only used by MailImpl to generate a new Mail name. >> I think there are two things which would make sense... >> >> 1) Remove the method from the interface and just move the Id >> generation code to MailImpl. In fact its only some kind of UUID and >> long combination >> 2) Create a new Interface called MailNameGenerator with the method >> createName() which will generate a new name to use in the Mail. >> >> I think I would prefer 1. > > I agree. > >> * MailServer.supportVirtualHosting(); >> This method returns true if the server supports virtualhosting. >> Which in fact means that the users must be added with an >> "@domainpart". So your users would look like: us...@domain1, >> us...@domain2 etc. >> So I think it would make more sense to move the method to >> UsersRepository interface. By default it would return false > > What about moving to virtual hosting as the default (and maybe the ONLY) > option? > When you setup a mailserver you have a domain, I think it is easier to > understand for newbies if they create users with their full email > address instead of using simple "usernames" and receive email for that > username on each configured domain. I thought about this too.. I'm just not sure if thats really the think todo. For example in qmail and postfix you don't have virtualhosting enabled by default. So I thought follow this princip make sense.. > >> * MailServer.getDefaultDomain(); >> Return the default domain which will get used if an email is >> delivered to an user via smtp and no domainpart was given. As this is >> clearly about domains I would suggest to move it to DomainList >> interface > > Make sense. > >> * MailServer.getHelloName(); >> Return the default hello name which will be used if no other is >> configured for the service. I would just remove this method at all and >> fallback to dnsservice.getLocalHost() by default if no hello name is >> configured for the service. > > I agree. THe hello name should be configured in the specific service, > doens't make sense to configure it in MailServer (it is not useful > there). > >> * MailServer.getPostMaster(): >> Is only used by MailetContext.getPostMaster() as a wrapped call. So >> just remove it. > > +1 > > Go ahead! > Stefano > Bye, Norman --------------------------------------------------------------------- To unsubscribe, e-mail: server-dev-unsubscr...@james.apache.org For additional commands, e-mail: server-dev-h...@james.apache.org