Re: resource loader API change
I agree that if it hadn't been B.C., it would have been a pretty important change, maybe legitimating a velocity2 package. But since ResourceLoader is just deprecated, and inherits from ResourceLoader2, this is *not* a backward-incompatible change. There are some incompatibilities, see http://velocity.apache.org/engine/devel/upgrading.html I tend to think they're not important enough to justify such a move. Claude On 16/07/2016 21:48, Nathan Bubna wrote: Not a bad idea. On Sat, Jul 16, 2016 at 12:31 PM, Sergiu Dumitriuwrote: I went on with the "2" suffix, but on the ResourceLoader class, hence a ResourceLoader2 class. Claude On 16/07/2016 10:54, Will Glass-Husain wrote: Makes sense to me. I'm always confused when names are a little similar but not identical. Can we just make a package called velocity2 or util2? Then keep the name the same ResourceLoader. It's a little more awkward sounding but is actually more understandable. Will On Saturday, July 16, 2016, Claude Brisson wrote: I recently commited a long awaited internal API change, letting resource loaders rely on Readers+encoding rather than on InputStreams (commits 1752784 and 1752787). As Nathan commented in VELOCITY-793, "Providing B.C. support would be nice, but is certainly not required nor even expected.". But I've got a little issue of conscience about how to handle backward compatibility here. We've got a method to deprecate or suppress: InputStream getResourceStream(String source) and a new method: Reader getResourceReader(String source, String encoding) For now, I handled B.C. by deprecating getResourceStream(), and implementing getResourceReader() to rely on the former one. It will serve the purpose of letting existing resource loaders work without any modification. But it has a big defect: someone implementing a new resource loader will typically want to just implement all absctract methods, which means he will have to implement getResourceStream(), and won't be asked to implement getResourceReader()... So I think we have to deprecate the ResourceLoader class itself, and create a new ResourceReader class. The new ResourceReader class will lack the getResourceStream() method. The deprecated ResourceLoader class will inherit ResourceReader, leave getResourceStream() abstract, and implement getResourceReader(). If you have another proposal to name the ResourceReader class, I'm interested. (ResourceFetcher? ResourceProvider?) Claude -- Sergiu Dumitriu http://purl.org/net/sergiu/ - To unsubscribe, e-mail: dev-unsubscr...@velocity.apache.org For additional commands, e-mail: dev-h...@velocity.apache.org - To unsubscribe, e-mail: dev-unsubscr...@velocity.apache.org For additional commands, e-mail: dev-h...@velocity.apache.org
Re: resource loader API change
Not a bad idea. On Sat, Jul 16, 2016 at 12:31 PM, Sergiu Dumitriuwrote: > If more backwards-incompatible changes are going to happen, maybe it's > best to do what several apache-commons project have done: move > everything to org.apache.velocity2. > > On 07/16/2016 09:11 AM, Nathan Bubna wrote: > > I like ResourceReader or ResourceProvider, as i get confused when classes > > have the same name and different packages. :) But i suppose > > ResourceLoader2 in all its ugliness solves both Will's confusion and my > > own. Ha! > > > > But in all seriousness, you know i'm a big "them that do the work make > the > > call" type of guy. I trust your judgement. > > > > On Sat, Jul 16, 2016 at 2:27 AM, Claude Brisson > wrote: > > > >> I went on with the "2" suffix, but on the ResourceLoader class, hence a > >> ResourceLoader2 class. > >> > >> Claude > >> > >> > >> On 16/07/2016 10:54, Will Glass-Husain wrote: > >> > >>> Makes sense to me. > >>> > >>> I'm always confused when names are a little similar but not identical. > Can > >>> we just make a package called velocity2 or util2? Then keep the name > the > >>> same ResourceLoader. It's a little more awkward sounding but is > actually > >>> more understandable. > >>> > >>> Will > >>> > >>> > >>> > >>> On Saturday, July 16, 2016, Claude Brisson wrote: > >>> > >>> I recently commited a long awaited internal API change, letting > resource > loaders rely on Readers+encoding rather than on InputStreams > (commits 1752784 and 1752787). > > As Nathan commented in VELOCITY-793, "Providing B.C. support would be > nice, but is certainly not required nor even expected.". But I've got > a > little issue of conscience about how to handle backward compatibility > here. > > We've got a method to deprecate or suppress: > > InputStream getResourceStream(String source) > > and a new method: > > Reader getResourceReader(String source, String encoding) > > For now, I handled B.C. by deprecating getResourceStream(), and > implementing getResourceReader() to rely on the former one. It will > serve > the purpose of letting existing resource loaders work without any > modification. > > But it has a big defect: someone implementing a new resource loader > will > typically want to just implement all absctract methods, which means he > will > have to implement getResourceStream(), and won't be asked to implement > getResourceReader()... > > So I think we have to deprecate the ResourceLoader class itself, and > create a new ResourceReader class. > > The new ResourceReader class will lack the getResourceStream() method. > The > deprecated ResourceLoader class will inherit ResourceReader, leave > getResourceStream() abstract, and implement getResourceReader(). > > If you have another proposal to name the ResourceReader class, I'm > interested. (ResourceFetcher? ResourceProvider?) > > > Claude > > > > > -- > Sergiu Dumitriu > http://purl.org/net/sergiu/ > > - > To unsubscribe, e-mail: dev-unsubscr...@velocity.apache.org > For additional commands, e-mail: dev-h...@velocity.apache.org > >
Re: resource loader API change
If more backwards-incompatible changes are going to happen, maybe it's best to do what several apache-commons project have done: move everything to org.apache.velocity2. On 07/16/2016 09:11 AM, Nathan Bubna wrote: > I like ResourceReader or ResourceProvider, as i get confused when classes > have the same name and different packages. :) But i suppose > ResourceLoader2 in all its ugliness solves both Will's confusion and my > own. Ha! > > But in all seriousness, you know i'm a big "them that do the work make the > call" type of guy. I trust your judgement. > > On Sat, Jul 16, 2016 at 2:27 AM, Claude Brissonwrote: > >> I went on with the "2" suffix, but on the ResourceLoader class, hence a >> ResourceLoader2 class. >> >> Claude >> >> >> On 16/07/2016 10:54, Will Glass-Husain wrote: >> >>> Makes sense to me. >>> >>> I'm always confused when names are a little similar but not identical. Can >>> we just make a package called velocity2 or util2? Then keep the name the >>> same ResourceLoader. It's a little more awkward sounding but is actually >>> more understandable. >>> >>> Will >>> >>> >>> >>> On Saturday, July 16, 2016, Claude Brisson wrote: >>> >>> I recently commited a long awaited internal API change, letting resource loaders rely on Readers+encoding rather than on InputStreams (commits 1752784 and 1752787). As Nathan commented in VELOCITY-793, "Providing B.C. support would be nice, but is certainly not required nor even expected.". But I've got a little issue of conscience about how to handle backward compatibility here. We've got a method to deprecate or suppress: InputStream getResourceStream(String source) and a new method: Reader getResourceReader(String source, String encoding) For now, I handled B.C. by deprecating getResourceStream(), and implementing getResourceReader() to rely on the former one. It will serve the purpose of letting existing resource loaders work without any modification. But it has a big defect: someone implementing a new resource loader will typically want to just implement all absctract methods, which means he will have to implement getResourceStream(), and won't be asked to implement getResourceReader()... So I think we have to deprecate the ResourceLoader class itself, and create a new ResourceReader class. The new ResourceReader class will lack the getResourceStream() method. The deprecated ResourceLoader class will inherit ResourceReader, leave getResourceStream() abstract, and implement getResourceReader(). If you have another proposal to name the ResourceReader class, I'm interested. (ResourceFetcher? ResourceProvider?) Claude -- Sergiu Dumitriu http://purl.org/net/sergiu/ - To unsubscribe, e-mail: dev-unsubscr...@velocity.apache.org For additional commands, e-mail: dev-h...@velocity.apache.org
Re: resource loader API change
I like ResourceReader or ResourceProvider, as i get confused when classes have the same name and different packages. :) But i suppose ResourceLoader2 in all its ugliness solves both Will's confusion and my own. Ha! But in all seriousness, you know i'm a big "them that do the work make the call" type of guy. I trust your judgement. On Sat, Jul 16, 2016 at 2:27 AM, Claude Brissonwrote: > I went on with the "2" suffix, but on the ResourceLoader class, hence a > ResourceLoader2 class. > > Claude > > > On 16/07/2016 10:54, Will Glass-Husain wrote: > >> Makes sense to me. >> >> I'm always confused when names are a little similar but not identical. Can >> we just make a package called velocity2 or util2? Then keep the name the >> same ResourceLoader. It's a little more awkward sounding but is actually >> more understandable. >> >> Will >> >> >> >> On Saturday, July 16, 2016, Claude Brisson wrote: >> >> I recently commited a long awaited internal API change, letting resource >>> loaders rely on Readers+encoding rather than on InputStreams >>> (commits 1752784 and 1752787). >>> >>> As Nathan commented in VELOCITY-793, "Providing B.C. support would be >>> nice, but is certainly not required nor even expected.". But I've got a >>> little issue of conscience about how to handle backward compatibility >>> here. >>> >>> We've got a method to deprecate or suppress: >>> >>> InputStream getResourceStream(String source) >>> >>> and a new method: >>> >>>Reader getResourceReader(String source, String encoding) >>> >>> For now, I handled B.C. by deprecating getResourceStream(), and >>> implementing getResourceReader() to rely on the former one. It will serve >>> the purpose of letting existing resource loaders work without any >>> modification. >>> >>> But it has a big defect: someone implementing a new resource loader will >>> typically want to just implement all absctract methods, which means he >>> will >>> have to implement getResourceStream(), and won't be asked to implement >>> getResourceReader()... >>> >>> So I think we have to deprecate the ResourceLoader class itself, and >>> create a new ResourceReader class. >>> >>> The new ResourceReader class will lack the getResourceStream() method. >>> The >>> deprecated ResourceLoader class will inherit ResourceReader, leave >>> getResourceStream() abstract, and implement getResourceReader(). >>> >>> If you have another proposal to name the ResourceReader class, I'm >>> interested. (ResourceFetcher? ResourceProvider?) >>> >>> >>>Claude >>> >>> > > - > To unsubscribe, e-mail: dev-unsubscr...@velocity.apache.org > For additional commands, e-mail: dev-h...@velocity.apache.org > >
Re: resource loader API change
I went on with the "2" suffix, but on the ResourceLoader class, hence a ResourceLoader2 class. Claude On 16/07/2016 10:54, Will Glass-Husain wrote: Makes sense to me. I'm always confused when names are a little similar but not identical. Can we just make a package called velocity2 or util2? Then keep the name the same ResourceLoader. It's a little more awkward sounding but is actually more understandable. Will On Saturday, July 16, 2016, Claude Brissonwrote: I recently commited a long awaited internal API change, letting resource loaders rely on Readers+encoding rather than on InputStreams (commits 1752784 and 1752787). As Nathan commented in VELOCITY-793, "Providing B.C. support would be nice, but is certainly not required nor even expected.". But I've got a little issue of conscience about how to handle backward compatibility here. We've got a method to deprecate or suppress: InputStream getResourceStream(String source) and a new method: Reader getResourceReader(String source, String encoding) For now, I handled B.C. by deprecating getResourceStream(), and implementing getResourceReader() to rely on the former one. It will serve the purpose of letting existing resource loaders work without any modification. But it has a big defect: someone implementing a new resource loader will typically want to just implement all absctract methods, which means he will have to implement getResourceStream(), and won't be asked to implement getResourceReader()... So I think we have to deprecate the ResourceLoader class itself, and create a new ResourceReader class. The new ResourceReader class will lack the getResourceStream() method. The deprecated ResourceLoader class will inherit ResourceReader, leave getResourceStream() abstract, and implement getResourceReader(). If you have another proposal to name the ResourceReader class, I'm interested. (ResourceFetcher? ResourceProvider?) Claude - To unsubscribe, e-mail: dev-unsubscr...@velocity.apache.org For additional commands, e-mail: dev-h...@velocity.apache.org
Re: resource loader API change
Makes sense to me. I'm always confused when names are a little similar but not identical. Can we just make a package called velocity2 or util2? Then keep the name the same ResourceLoader. It's a little more awkward sounding but is actually more understandable. Will On Saturday, July 16, 2016, Claude Brissonwrote: > I recently commited a long awaited internal API change, letting resource > loaders rely on Readers+encoding rather than on InputStreams > (commits 1752784 and 1752787). > > As Nathan commented in VELOCITY-793, "Providing B.C. support would be > nice, but is certainly not required nor even expected.". But I've got a > little issue of conscience about how to handle backward compatibility here. > > We've got a method to deprecate or suppress: > >InputStream getResourceStream(String source) > > and a new method: > > Reader getResourceReader(String source, String encoding) > > For now, I handled B.C. by deprecating getResourceStream(), and > implementing getResourceReader() to rely on the former one. It will serve > the purpose of letting existing resource loaders work without any > modification. > > But it has a big defect: someone implementing a new resource loader will > typically want to just implement all absctract methods, which means he will > have to implement getResourceStream(), and won't be asked to implement > getResourceReader()... > > So I think we have to deprecate the ResourceLoader class itself, and > create a new ResourceReader class. > > The new ResourceReader class will lack the getResourceStream() method. The > deprecated ResourceLoader class will inherit ResourceReader, leave > getResourceStream() abstract, and implement getResourceReader(). > > If you have another proposal to name the ResourceReader class, I'm > interested. (ResourceFetcher? ResourceProvider?) > > > Claude >
resource loader API change
I recently commited a long awaited internal API change, letting resource loaders rely on Readers+encoding rather than on InputStreams (commits 1752784 and 1752787). As Nathan commented in VELOCITY-793, "Providing B.C. support would be nice, but is certainly not required nor even expected.". But I've got a little issue of conscience about how to handle backward compatibility here. We've got a method to deprecate or suppress: InputStream getResourceStream(String source) and a new method: Reader getResourceReader(String source, String encoding) For now, I handled B.C. by deprecating getResourceStream(), and implementing getResourceReader() to rely on the former one. It will serve the purpose of letting existing resource loaders work without any modification. But it has a big defect: someone implementing a new resource loader will typically want to just implement all absctract methods, which means he will have to implement getResourceStream(), and won't be asked to implement getResourceReader()... So I think we have to deprecate the ResourceLoader class itself, and create a new ResourceReader class. The new ResourceReader class will lack the getResourceStream() method. The deprecated ResourceLoader class will inherit ResourceReader, leave getResourceStream() abstract, and implement getResourceReader(). If you have another proposal to name the ResourceReader class, I'm interested. (ResourceFetcher? ResourceProvider?) Claude