Re: [VOTE]Support package scope via `package` keyword

[Paul King]

On Fri, Dec 29, 2017 at 9:35 PM, Jochen Theodorou  wrote:

> On 29.12.2017 11:58, Paul King wrote:
> [...]
>
>> I am unsure if I made myself clear. I think the default should remain as
>> CLASS but there could be an additional ALL enum value. Then again we could
>> just have a PackageScopeTarget[] ALL constant (though we have an
>> outstanding issue around using constants in annotation attributes we'd have
>> to check didn't get in the way).
>>
>
> It doesn´t make sense on local variables, the package, parameters or
> parameter types... Maybe I misunderstood what you mean with "ALL"
>

When you annotate a class like this:

```
import groovy.transform.PackageScope
import static groovy.transform.PackageScopeTarget.METHODS

@PackageScope(METHODS)
class Foo {
def method1() {}
def method2() {}
def method3() {}
}
```

Then, the three methods are package private and the class is public.

You can alternatively have: @PackageScope([METHODS, CONSTRUCTORS]) or
whatever combinations
you like. For a class with many methods, fields, constructors this is much
more concise than an extra keyword on each member.
I was suggesting ALL as a constant [METHODS, CONSTRUCTORS, FIELDS, CLASS].
Like Cédric, I am not a huge fan of using just package as the keyword and
if we use `package private`, then deprecating the
shorthand variant could lead to ugly verbose code.

Cheers, Paul.


> bye Jochen
>

Re: [VOTE]Support package scope via `package` keyword

[Cédric Champeau]

I'd tend to be -1 on the name "package" and 0 on the feature. The -1 is
because "package" is not a modifier by itself. I'd prefer "package private"
(the official Java name for this). So it leads to the 0, because "package
private" is kind of verbose, and doesn't save much from "PackageScope".
However, it drives me to another topic that is related and that we talked
about a few years back: having the ability to use annotations without the
`@`. So you would say `compilestatic class Foo`, or `packagescope Foo foo`.

2017-12-29 12:35 GMT+01:00 Jochen Theodorou :

> On 29.12.2017 11:58, Paul King wrote:
> [...]
>
>> I am unsure if I made myself clear. I think the default should remain as
>> CLASS but there could be an additional ALL enum value. Then again we could
>> just have a PackageScopeTarget[] ALL constant (though we have an
>> outstanding issue around using constants in annotation attributes we'd have
>> to check didn't get in the way).
>>
>
> It doesn´t make sense on local variables, the package, parameters or
> parameter types... Maybe I misunderstood what you mean with "ALL"
>
>
> bye Jochen
>

Re: [VOTE]Support package scope via `package` keyword

[Daniel.Sun]

Hi Jochen,

  Thanks for your voting :-)

> but under the condition to deprecate @PackageScope in 2.6.0 and then I
> would like to have it removed in 3.0.0 (if we want to be serious about
> cleanups, that is) 

   Yeah, we can do better step by step.

Cheers,
Daniel.Sun



--
Sent from: http://groovy.329449.n5.nabble.com/Groovy-Dev-f372993.html

Re: [VOTE]Support package scope via `package` keyword

[Jochen Theodorou]

On 29.12.2017 11:58, Paul King wrote:
[...]
I am unsure if I made myself clear. I think the default should remain as 
CLASS but there could be an additional ALL enum value. Then again we 
could just have a PackageScopeTarget[] ALL constant (though we have an 
outstanding issue around using constants in annotation attributes we'd 
have to check didn't get in the way).


It doesn´t make sense on local variables, the package, parameters or 
parameter types... Maybe I misunderstood what you mean with "ALL"



bye Jochen

Re: [VOTE]Support package scope via `package` keyword

[Jochen Theodorou]

On 29.12.2017 03:40, Daniel Sun wrote:

Hi all,

   In order to support package scope, we have to add annotation
`@PackageScope` to the target(e.g. class, field, method), which is verbose.
Nathan Harvey has started a thread to discuss specifying the package scope
via `package` keyword[1]. The feature is feasible and has been implemented
in my labs project[2].


Just to be clear, this feature, this is a change for IDEs, not for the 
user directly. For example a project I am working on with like 90k lines 
of code, I think I used package private 3 times...no 2 times, because 
the other got changed.



   Please vote on add the new feature to Apache Groovy 3.0.0 and 2.6.0
releases.

   The vote is open for the next 72 hours and passes if a majority of at
least three +1 PMC votes are cast.

[ ] +1  The feature sounds good
[ ]   0  I don't have a strong opinion about this, but I assume it's ok
[ ]  -1  Because...


You get a +1 from me, but under the condition to deprecate @PackageScope 
in 2.6.0 and then I would like to have it removed in 3.0.0 (if we want 
to be serious about cleanups, that is)


bye Jochen

Re: [VOTE]Support package scope via `package` keyword

[Paul King]

On Fri, Dec 29, 2017 at 8:34 PM, Daniel.Sun  wrote:

> Hi Paul,
>
> > Just so I understand, the intention is to keep @PackageScope with
> > `package` being an alias for when no target is required. Correct?
>
> Yeah, correct :-)
>
> > Come to think of it, I wonder if there should be an `ALL` target?
> I agree with you that the target of @PackageScope should be `ALL`
>


I am unsure if I made myself clear. I think the default should remain as
CLASS but there could be an additional ALL enum value. Then again we could
just have a PackageScopeTarget[] ALL constant (though we have an
outstanding issue around using constants in annotation attributes we'd have
to check didn't get in the way).


> your vote is ?
>

I am at least a +0, just pondering the real merit given that it doesn't
really save much work for IDE writers and it won't be more concise for a
range of typical use cases. It doesn't scream a +1 for me but I'll ponder
for a few more days. I'm keen to hear more thoughts.

Cheers, Paul.


>
> Cheers,
> Daniel.Sun
>
>
>
> --
> Sent from: http://groovy.329449.n5.nabble.com/Groovy-Dev-f372993.html
>

Re: [VOTE]Support package scope via `package` keyword

[Daniel.Sun]

Hi Paul,

> Just so I understand, the intention is to keep @PackageScope with
> `package` being an alias for when no target is required. Correct?

Yeah, correct :-)

> Come to think of it, I wonder if there should be an `ALL` target?
I agree with you that the target of @PackageScope should be `ALL`

your vote is ?

Cheers,
Daniel.Sun



--
Sent from: http://groovy.329449.n5.nabble.com/Groovy-Dev-f372993.html

Re: [VOTE]Support package scope via `package` keyword

[Paul King]

Just so I understand, the intention is to keep @PackageScope with `package`
being an alias for when no target is required. Correct?

If that's correct, it won't eliminate the need for IDEs to scan for
@PackageScope in general but in some common cases, some preliminary
assumptions could be made.

Also, when documenting, we should note that `package` will be more concise
when used in one or two places within a class but @PackageScope will be
more concise if used with a PackageScopeTarget when a few (or more) members
should be made package private. Come to think of it, I wonder if there
should be an `ALL` target?

Paul.

On Fri, Dec 29, 2017 at 12:40 PM, Daniel Sun 
wrote:

> Hi all,
>
>   In order to support package scope, we have to add annotation
> `@PackageScope` to the target(e.g. class, field, method), which is verbose.
> Nathan Harvey has started a thread to discuss specifying the package scope
> via `package` keyword[1]. The feature is feasible and has been implemented
> in my labs project[2].
>
>   Please vote on add the new feature to Apache Groovy 3.0.0 and 2.6.0
> releases.
>
>   The vote is open for the next 72 hours and passes if a majority of at
> least three +1 PMC votes are cast.
>
> [ ] +1  The feature sounds good
> [ ]   0  I don't have a strong opinion about this, but I assume it's ok
> [ ]  -1  Because...
>
> Cheers,
> Daniel.Sun
>
> [1]
> http://groovy.329449.n5.nabble.com/Package-specific-syntax-tp5745613.html
> [2] https://github.com/danielsun1106/groovy-parser/tree/package-scope
>
>
>
>
> --
> Sent from: http://groovy.329449.n5.nabble.com/Groovy-Dev-f372993.html
>