Re: Code reformatting

2020-12-24 Thread Erick Erickson
I like braces too. For this kind of mass edit though, I’m not about to 
start changing anything that isn’t already touched by the formatter,
that’s a never-ending rat-hole and rife with possibilities to screw
up.

What that means in this case is that the formatter changes

if (something)
   blah

to 

if (something) blah

and it shows up in the diff.

Since the last person to deal with that code intended “blah” to be
on a separate line, I’ll add braces put “blah” back on it’s own line:

if (something) {
   blah
}

OTOH, since the formatter doesn’t touch

if (something) blah

I won’t either. I’m sure I’ll miss some, but since it’s functionally equivalent
I don’t care. BTW, I’ve even seen a couple of 

for (whatever) 
  blah;

which I think is horrible. Again, though, if the formatter touches it I’ll
put in braces, otherwise I won’t even see it.

Martin Fowler in his book “refactoring” argues that there’s no
point in rewriting code _unless_ it’s relevant to a problem you’re solving.
In this case that means something that’s being reformatted anyway….

FWIW,
Erick

> On Dec 24, 2020, at 1:40 PM, Gus Heck  wrote:
> 
> +1 to always braces
> 
> On Thu, Dec 24, 2020 at 11:47 AM Michael Sokolov  wrote:
> The Google convention you cited says this, I think?
> 
> >Braces are used with if, else, for, do and while statements, even when 
> > the body is empty or contains only a single statement.
> 
> On Thu, Dec 24, 2020 at 8:00 AM Dawid Weiss  wrote:
> >
> > > Personally I would ban the non block conditional, but I think it's moot 
> > > in this context since spotless just does what it does and is not 
> > > configurable, as I understand it. I suppose we could manually "fix" all 
> > > the conditionals though?
> >
> > I'm pretty sure you could do it automatically... But in many places
> > there is very little sense in doing that. That google format
> > convention [1] is fairly reasonable to me - strict in certain aspects
> > and relaxed elsewhere. I wouldn't enforce it. If you find a place that
> > could use more clarity with braces, correct it (and re-run the
> > formatting) then commit it back in.
> >
> > Dawid
> >
> > https://google.github.io/styleguide/javaguide.html
> >
> > -
> > To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
> > For additional commands, e-mail: dev-h...@lucene.apache.org
> >
> 
> -
> To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
> For additional commands, e-mail: dev-h...@lucene.apache.org
> 
> 
> 
> -- 
> http://www.needhamsoftware.com (work)
> http://www.the111shift.com (play)


-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org



Re: Code reformatting

2020-12-24 Thread Gus Heck
+1 to always braces

On Thu, Dec 24, 2020 at 11:47 AM Michael Sokolov  wrote:

> The Google convention you cited says this, I think?
>
> >Braces are used with if, else, for, do and while statements, even
> when the body is empty or contains only a single statement.
>
> On Thu, Dec 24, 2020 at 8:00 AM Dawid Weiss  wrote:
> >
> > > Personally I would ban the non block conditional, but I think it's
> moot in this context since spotless just does what it does and is not
> configurable, as I understand it. I suppose we could manually "fix" all the
> conditionals though?
> >
> > I'm pretty sure you could do it automatically... But in many places
> > there is very little sense in doing that. That google format
> > convention [1] is fairly reasonable to me - strict in certain aspects
> > and relaxed elsewhere. I wouldn't enforce it. If you find a place that
> > could use more clarity with braces, correct it (and re-run the
> > formatting) then commit it back in.
> >
> > Dawid
> >
> > https://google.github.io/styleguide/javaguide.html
> >
> > -
> > To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
> > For additional commands, e-mail: dev-h...@lucene.apache.org
> >
>
> -
> To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
> For additional commands, e-mail: dev-h...@lucene.apache.org
>
>

-- 
http://www.needhamsoftware.com (work)
http://www.the111shift.com (play)


Re: Code reformatting

2020-12-24 Thread Michael Sokolov
The Google convention you cited says this, I think?

>Braces are used with if, else, for, do and while statements, even when the 
> body is empty or contains only a single statement.

On Thu, Dec 24, 2020 at 8:00 AM Dawid Weiss  wrote:
>
> > Personally I would ban the non block conditional, but I think it's moot in 
> > this context since spotless just does what it does and is not configurable, 
> > as I understand it. I suppose we could manually "fix" all the conditionals 
> > though?
>
> I'm pretty sure you could do it automatically... But in many places
> there is very little sense in doing that. That google format
> convention [1] is fairly reasonable to me - strict in certain aspects
> and relaxed elsewhere. I wouldn't enforce it. If you find a place that
> could use more clarity with braces, correct it (and re-run the
> formatting) then commit it back in.
>
> Dawid
>
> https://google.github.io/styleguide/javaguide.html
>
> -
> To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
> For additional commands, e-mail: dev-h...@lucene.apache.org
>

-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org



Re: [DISCUSS] SIP-12: Incremental Backup and Restore

2020-12-24 Thread Jan Høydahl
Ok, that’s the one I was looking for, it’s not documented in the backup chapter 
of ref-guide :( 

Jan Høydahl

> 23. des. 2020 kl. 17:10 skrev Jason Gerlowski :
> 
> 
>> 
>> We have a path alias to the old API ... but we don’t have a true v2 API spec 
>> for it, do we?
> 
> Tbh I'm not yet familiar enough with the v2 APIs to understand the
> distinction you're making.  (Do you have a pointer to something that'd
> fill me in?)
> 
> To zoom in on "backup" as an example, the v2 API I'm referring to
> looks like:  /v2/collections" -d '{ "backup-collection":
> {"collection": "books", "name": "asdf3", "location": "/tmp/foo"}}'.
> And it's included in the v2 "introspect" documentation returned by
> this API: /v2/collections/_introspect?command=backup-collection".  To
> me that looked like a v2 API, but maybe path-aliases are also covered
> in the introspect docs.
> 
> Jason
> 
>> On Wed, Dec 23, 2020 at 10:29 AM Jan Høydahl  wrote:
>> 
>> Actually, don’t think we do have a v2 Backup/Restore API. We have a path 
>> alias to the old API which takes GET ...=backup... but we don’t have 
>> a true v2 API spec for it, do we? Where is that documented?
>> 
>> Jan Høydahl
>> 
 22. des. 2020 kl. 18:04 skrev Jason Gerlowski :
>>> 
>>> Hey guys,
>>> 
>>> Following up to make sure I understand the specifics you're
>>> suggesting.  You're proposing that:
>>> 
>>> 1. The brand new backup-related APIs (list-backups and delete-backup)
>>> be added in v2-form only.
>>> 2. Tweaks to existing backup-related APIs (create-backup, restore) be
>>> made in V2-form only.
>>> 3. All existing v1 backup-related APIs be deprecated and left
>>> unchanged.  Incremental backups will not be possible using the v1 API.
>>> 
>>> I'm not against going this route if there's consensus around it.  But
>>> I'm not 100% clear on how it means we don't need to worry about
>>> backcompat.  Backup and Restore currently exist as both a v1 and a v2
>>> API - I understand how leaving the v1 APIs untouched (other than
>>> deprecation) frees us of some backcompat concerns there, but we would
>>> still need to make tweaks to the v2 backup/restore APIs and would have
>>> to tread just as carefully there in terms of backcompat, afaict.
>>> Unless Solr's backcompatibility guarantees only cover the v1 API and
>>> leave v2 changes to be made freely?  I looked around to see if the v2
>>> APIs had any sort of "experimental" designation, but couldn't find
>>> that clearly stated anywhere.  Am I missing something?
>>> 
>>> Best,
>>> 
>>> Jason
>>> 
 On Tue, Dec 22, 2020 at 2:49 AM Noble Paul  wrote:
 
> , and implement the new imporved version as a V2-api only, and then 
> deprecate the v1 API?
 
 
 V2 only please
 
> On Tue, Dec 22, 2020 at 1:34 AM Jason Gerlowski  
> wrote:
> 
> Hey Jan, thanks for the review.
> 
> I hadn't thought about the V2 API in connection to this work.  You're
> right though I think - the SIP proposes net-new APIs, so it should add
> V2 equivalents at the very least.  I'll draft tentative details for
> these APIs on the SIP and we can refine things from there.
> 
> I'm more up in the air on your specific suggestion to restrict the SIP
> changes to these v2 APIs.  It is an elegant approach to the
> backcompat, and it provides a carrot for v2 adoption - both of which I
> like.  But it would let users create snapshot-based backups (and keep
> us maintaining that code) longer than there's any strict need to.  And
> users are left on the less-efficient format by default.  (By contrast,
> the current SIP has snapshot-backup creation being replaced by
> incremental-backup creation as soon as the latter is available.).  Did
> you have a particular lifespan in mind for snapshot-based creation if
> we go with this approach?
> 
> Jason
> 
> On Thu, Dec 17, 2020 at 3:54 PM Jan Høydahl  wrote:
>> 
>> Much needed! Thanks for initiating this Jason!
>> 
>> As we want to move away from v1 APIs where a HTTP GET is used for 
>> creation and deletion, would it be an idea to leave the old 
>> backup/resotre APIs as-is, and implement the new imporved version as a 
>> V2-api only, and then deprecate the v1 API? Then we don't need to worry 
>> about back-compat, and we get a head-start on converting the COLLECTION 
>> API to v2 style.
>> 
>> Jan
>> 
>>> 15. des. 2020 kl. 15:48 skrev Jason Gerlowski :
>>> 
>>> Hey all,
>>> 
>>> This morning I published SIP-12, which proposes an overhaul of Solr's
>>> backup and restore functionality.  While the "headline" improvement in
>>> this SIP is a change to do backups incrementally, it bundles in a
>>> number of other improvements as well, including the addition of
>>> corruption checks, APIs to list and delete backups, and stronger
>>> integration points with popular object storage APIs.
>>> 
>>> The SIP 

Re: Code reformatting

2020-12-24 Thread Dawid Weiss
> Personally I would ban the non block conditional, but I think it's moot in 
> this context since spotless just does what it does and is not configurable, 
> as I understand it. I suppose we could manually "fix" all the conditionals 
> though?

I'm pretty sure you could do it automatically... But in many places
there is very little sense in doing that. That google format
convention [1] is fairly reasonable to me - strict in certain aspects
and relaxed elsewhere. I wouldn't enforce it. If you find a place that
could use more clarity with braces, correct it (and re-run the
formatting) then commit it back in.

Dawid

https://google.github.io/styleguide/javaguide.html

-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org



[SOLR-15058] nodeName format in common.util.Utils#getBaseUrlForNodeName

2020-12-24 Thread sasa su
I am setting up Solr in some environment, those hostname looks like foo_bar. then I got this strange error.    org.apache.solr.client.solrj.SolrServerException:IOException occured when talking to server at: http://foo/bar:18984_solr Solr can not understand the hostname if it contains '_', the reason is the getBaseUrlForNodeName function. The logic of this function is - split the input into two part by first '_' character - URL decode the second part - return first part + '/' + second part Solr will understand the hostname if change the split point to the last '_', But will break this test: input:   127.0.0.1:61631_l_%2Fig output:  127.0.0.1:6131/l_/ig It looks like someone is already relying on this behavior, change this behavior will break some existed code.Erick advises the split point should be '_solr', and it will break more unit tests. I want to ask If someone depends on the behavior, and can I change the behavior?Change the split point to last '_' looks has a minimal break. If someone really depends on the behavior, I think the best way is to document the hostname must conform to RFC1035. PR at https://github.com/apache/lucene-solr/pull/2164