Re: Process of making changes at trunk
> On Apr 4, 2018, at 11:47 PM, Emmanuel Lécharny wrote: > > Le 05/04/2018 à 00:57, Shawn McKinney a écrit : >> >>> On Apr 4, 2018, at 4:18 AM, Emmanuel Lécharny wrote: >>> >>> One more thing : >>> >>> if ( props.size() > 0 ) >>> >>> should be >>> >>> if ( !props.isEmpty() ) >> >> That won’t work, props is not java.util.Properties, it’s: > >List props = inProps.getEntry(); > > So it *must* implement List.isEmpty()... I stand corrected. Thanks
Re: Process of making changes at trunk
Le 05/04/2018 à 00:57, Shawn McKinney a écrit : > >> On Apr 4, 2018, at 4:18 AM, Emmanuel Lécharny wrote: >> >> One more thing : >> >> if ( props.size() > 0 ) >> >> should be >> >> if ( !props.isEmpty() ) > > That won’t work, props is not java.util.Properties, it’s: List props = inProps.getEntry(); So it *must* implement List.isEmpty()... -- Emmanuel Lecharny Symas.com directory.apache.org
Re: Process of making changes at trunk
> On Apr 4, 2018, at 4:18 AM, Emmanuel Lécharny wrote:
>
> One more thing :
>
> if ( props.size() > 0 )
>
> should be
>
> if ( !props.isEmpty() )
That won’t work, props is not java.util.Properties, it’s:
public class Props extends FortEntity implements Serializable
{
/** Default serialVersionUID */
private static final long serialVersionUID = 1L;
private List entry;
Sort of a container class to carry properties as a list of name/values when
it’s time to serialize via jaxb. The rationale for converting escapes me atm,
other than it (serialization) didn’t work with attribute type of
java.util.Properties.
Might be worth a revisit, to clean this up, i.e. use the java.util version and
not this other one. We’d be able to also remove the redundant code Yudhi
mentioned, i.e. using the delete key.
Shawn
Re: Process of making changes at trunk
> On Apr 4, 2018, at 2:26 AM, Yudhi Karunia Surtan wrote: > > Currently we have RestUtils.getProperties What do you think if above > classes using RestUtils method to remove duplication? However, if we do > that from the dependency point of view, i think it is a bit awkward because > "model" package depends on "rest" package. Agreed, how about we create a sep utils in the model package and move the code into there?
Re: Process of making changes at trunk
Le 04/04/2018 à 09:26, Yudhi Karunia Surtan a écrit :
> Guys,
>
> Finally make my first commit to apache git repository.
> Thanks for guide me until here.
>
>
> I also find there are some duplication for some code like this :
>
> Properties properties = null;
> List props = inProps.getEntry();
> if ( props.size() > 0 )
> {
> properties = new Properties();
> //int size = props.size();
> for ( Props.Entry entry : props )
> {
> String key = entry.getKey();
> String val = entry.getValue();
> properties.setProperty( key, val );
> }
> }
> return properties;
>
>
> at org.apache.directory.fortress.core.model.Group,
> org.apache.directory.fortress.core.model.Permission,
> org.apache.directory.fortress.core.model.PermObj,
> org.apache.directory.fortress.core.model.Role,
> org.apache.directory.fortress.core.model.User.
>
> Currently we have RestUtils.getProperties What do you think if above
> classes using RestUtils method to remove duplication? However, if we do
> that from the dependency point of view, i think it is a bit awkward because
> "model" package depends on "rest" package.
I do think an utility function should be added to fortress-core, as each
project depends on it. It makes no sense to move that to the
fortress-rest module, IMHO.
One more thing :
if ( props.size() > 0 )
should be
if ( !props.isEmpty() )
And, yes, +1 to remove code duplication :-)
--
Emmanuel Lecharny
Symas.com
directory.apache.org
Re: Process of making changes at trunk
Guys,
Finally make my first commit to apache git repository.
Thanks for guide me until here.
I also find there are some duplication for some code like this :
Properties properties = null;
List props = inProps.getEntry();
if ( props.size() > 0 )
{
properties = new Properties();
//int size = props.size();
for ( Props.Entry entry : props )
{
String key = entry.getKey();
String val = entry.getValue();
properties.setProperty( key, val );
}
}
return properties;
at org.apache.directory.fortress.core.model.Group,
org.apache.directory.fortress.core.model.Permission,
org.apache.directory.fortress.core.model.PermObj,
org.apache.directory.fortress.core.model.Role,
org.apache.directory.fortress.core.model.User.
Currently we have RestUtils.getProperties What do you think if above
classes using RestUtils method to remove duplication? However, if we do
that from the dependency point of view, i think it is a bit awkward because
"model" package depends on "rest" package.
On Tue, Feb 13, 2018 at 10:09 PM, Shawn McKinney
wrote:
>
> > On Feb 13, 2018, at 9:06 AM, Shawn McKinney
> wrote:
> >
> > I always create an issue in JIRA, with a detailed description of the
> problem and the fix, and use the jira issue #, along with a short desc, in
> the commit comments.
> >
> > e.g. FC-231 - DAO's should be package private
> >
> > That way we can tie the change back to the issue, where longer
> description of the change may take place.
>
> Another advantage of using JIRA to manage issues, we have a nice list of
> changes, for the release notes.
Re: Process of making changes at trunk
> On Feb 13, 2018, at 9:06 AM, Shawn McKinney wrote: > > I always create an issue in JIRA, with a detailed description of the problem > and the fix, and use the jira issue #, along with a short desc, in the commit > comments. > > e.g. FC-231 - DAO's should be package private > > That way we can tie the change back to the issue, where longer description of > the change may take place. Another advantage of using JIRA to manage issues, we have a nice list of changes, for the release notes.
Re: Process of making changes at trunk
> On Feb 12, 2018, at 9:14 PM, Yudhi Karunia Surtan > wrote: > > Do I need to create an issue and create a new branch, or just commit the > changes to the master branch? Hi Yudhi, Typically when we make a change, we’ll notify the project about the issue, the fix, etc, (which you have already done). I always create an issue in JIRA, with a detailed description of the problem and the fix, and use the jira issue #, along with a short desc, in the commit comments. e.g. FC-231 - DAO's should be package private That way we can tie the change back to the issue, where longer description of the change may take place. For something trivial, like a code comment, or some other self-explanatory change, I will skip the step to create the JIRA issue, and just commit, with the short comment. For really disruptive changes, i.e. changing the API, data structures, we discuss on this ML the pros/cons, reach consensus, before going forward. In these cases we might create a branch, because of the duration it takes to complete the work. For simple changes, like that which you’ve brought up now, simply committing directly to trunk is fine. One word of caution, I always make sure the junit tests pass successfully before checking in a change. The idea if the tests pass, no regressions, or other known bugs have been inserted. If the tests pass, and a bug is introduce anyway, we know its time to add a new test case. Let me know if you have any questions about how to do this, which tests, etc. Good questions btw, please feel welcomed to continue asking these questions here. One of these days I’m going to document these procedures, for our coding handbook, in the meantime, apologize for the tribal knowledge. :-) Shawn
Re: Process of making changes at trunk
Hi Stefan, Ah, I see.. Thanks for your answer. Do I need to create an issue and create a new branch, or just commit the changes to the master branch? On Feb 13, 2018 1:55 AM, "Stefan Seelmann" wrote: > Hi Yudhi, > > On 02/12/2018 07:32 PM, Yudhi Karunia Surtan wrote: > > Greetings, > > > > I'm planning to make a change for fortress rest util connection pooling > > mechanism. > > Currently we only used 2 connections for the connection pooling as the > > default value provided by apache http client library. > > Apart from the connection pool, i also look that it would be good if we > > could manage the heap memory management in string using StringUtils.join > > method. > > > > Since that library used and simplify the StringBuilder mechanism and the > > most benefit is to maintain the GC time become very minimum. I know from > > the performance point of view StringBuilder will make a bit slow compare > to > > the concatenation one. > > However, if we considering the greater good of GC, StringBuilder can > reduce > > the old GC period since the objects will be discarded before it reach old > > GC because it will not wait until the strong string reference got > destroyed. > > > > As per my github pull request shawn asked me to put that concern at the > > mailing list, unfortunatelly i can't find the exact svn at > > https://svn.apache.org/viewvc/directory/trunks/ for fortress project. > > Please guide how to commit my code contribution. > > Fortress already uses git, not SVN, you can find the repository URLs > here: https://directory.apache.org/sources.html You can just git clone > those and then push via HTTPS using your Apache user and password. > > Fortress still uses the old git-wip-us service, it would make sense to > move it to gitbox which would then allow bidirectional sync with GitHub. > > Kind Regards, > Stefan >
Re: Process of making changes at trunk
Hi Yudhi, On 02/12/2018 07:32 PM, Yudhi Karunia Surtan wrote: > Greetings, > > I'm planning to make a change for fortress rest util connection pooling > mechanism. > Currently we only used 2 connections for the connection pooling as the > default value provided by apache http client library. > Apart from the connection pool, i also look that it would be good if we > could manage the heap memory management in string using StringUtils.join > method. > > Since that library used and simplify the StringBuilder mechanism and the > most benefit is to maintain the GC time become very minimum. I know from > the performance point of view StringBuilder will make a bit slow compare to > the concatenation one. > However, if we considering the greater good of GC, StringBuilder can reduce > the old GC period since the objects will be discarded before it reach old > GC because it will not wait until the strong string reference got destroyed. > > As per my github pull request shawn asked me to put that concern at the > mailing list, unfortunatelly i can't find the exact svn at > https://svn.apache.org/viewvc/directory/trunks/ for fortress project. > Please guide how to commit my code contribution. Fortress already uses git, not SVN, you can find the repository URLs here: https://directory.apache.org/sources.html You can just git clone those and then push via HTTPS using your Apache user and password. Fortress still uses the old git-wip-us service, it would make sense to move it to gitbox which would then allow bidirectional sync with GitHub. Kind Regards, Stefan
Process of making changes at trunk
Greetings, I'm planning to make a change for fortress rest util connection pooling mechanism. Currently we only used 2 connections for the connection pooling as the default value provided by apache http client library. Apart from the connection pool, i also look that it would be good if we could manage the heap memory management in string using StringUtils.join method. Since that library used and simplify the StringBuilder mechanism and the most benefit is to maintain the GC time become very minimum. I know from the performance point of view StringBuilder will make a bit slow compare to the concatenation one. However, if we considering the greater good of GC, StringBuilder can reduce the old GC period since the objects will be discarded before it reach old GC because it will not wait until the strong string reference got destroyed. As per my github pull request shawn asked me to put that concern at the mailing list, unfortunatelly i can't find the exact svn at https://svn.apache.org/viewvc/directory/trunks/ for fortress project. Please guide how to commit my code contribution.
