Re: [DISCUSS] When to run CheckStyle and other verificiations
So given all the feedback, I'm going to do the following: "jar" will depend on "check" target "build-project", "build-test" and "test" targets will not depend on "check" target "check" target will include checkstyle, rat and eclipse-warnings There is an additional flag "no-check" to disable checks in the "jar" target. Will not introduce any Git hook. wt., 27 cze 2023 o 18:35 Jacek Lewandowski napisał(a): > With git you can always opt-out by adding --no-verify flag to either push > or commit > > I really prefer separate tasks than flags. Flags are not listed in the > help message like "ant -p" and are not auto-completed in the terminal. That > makes them almost undiscoverable for newcomers. > > Want to have jar include checks? Ok, but let's don't run checks > automatically with "build" or "test" > > > > wt., 27 cze 2023 o 18:26 David Capwell napisał(a): > >> nobody referred to running checks in a pre-push (or pre-commit) hook >> >> >> >> In accord I added an opt-out for each hook, and will require such here as >> well… as long as you can opt-out, its fine by me… I know I will likely >> opt-out, but wouldn’t block such an effort >> >> Your point that pre-push hook might not be the best one is valid, and we >> should rather think about pre-commit >> >> >> Pre-push is far better than pre-commit, with pre-commit you are forcing a >> style on people…. I for one have many many commits in a single PR, where I >> use commits to not loose track of progress (even if the code doesn’t >> compile), so forcing the build to work would be a -1 from me…. Pre-push at >> least means “you want the world to see this” so makes more sense there… >> >> Again, must have an opt-out >> >> proposed: >> ant jar (just build) >> git commit (run some checks) >> >> >> I am against this, jar should also check and ask users to opt-out if they >> don’t want the checks…. >> >> If we go with opt-out i'd like to see one flag that can disable all checks: >> `-Dchecks.skip` >> >> >> Works for me, you can also do the following to disable and not worry about >> >> $ cat < build.properties >> checks.skip: true >> EOF >> >> On Jun 27, 2023, at 3:14 AM, Mick Semb Wever wrote: >> >> The context is that we currently have 3 checks in the build: >> >> - Checkstyle, >> - Eclipse-Warnings, >> - RAT >> >> >> >> And dependency-check (owasp). >> >> >> >> I want to discuss whether you are ok with extracting all checks to their >> distinct target and not running it automatically with the targets which >> devs usually run locally. In particular: >> >> >> "build", "jar", and all "test" targets would not trigger CheckStyle, RAT >> or Eclipse-Warnings >> A new target "check" would trigger all CheckStyle, RAT, and >> Eclipse-Warnings >> The new "check" target would be run along with the "artifacts" target on >> Jenkins-CI, and it as a separate build step in CircleCI >> >> >> >> +0 I prefer this opt-in over an opt-out approach. >> >> It should be separated from `artifacts` too. >> We would need to encourage engineers to run `ant check` before >> starting CI and/or requesting review. >> >> I'm in favour of the opt-in approach because it keeps it visible. >> Folks configure flags and it "disappears" forever. Also it's a >> headache in all the other ant targets where we actually don't want it, >> e.g. tests. >> >> If we go with opt-out i'd like to see one flag that can disable all >> checks: `-Dchecks.skip` >> >> >> That could be fixed by running checks in a pre-push Git hook. There are >> some benefits of this compared to the current behavior: >> >> >> >> -1 >> committing and pushing to a personal branch is often done to save work >> or for cross-machine or collaboration. We should not gate on checks or >> compilation here. >> >> PRs should fail if checks fail, to give newcomers clear feedback (and >> to take this nit-picking out of the review process). >> >> >>
Re: [DISCUSS] When to run CheckStyle and other verificiations
This sounds good to me. Can we shorten 'build-project' to just 'build'? Kind Regards, Brandon On Thu, Jun 29, 2023 at 3:22 AM Jacek Lewandowski wrote: > > So given all the feedback, I'm going to do the following: > > "jar" will depend on "check" target > "build-project", "build-test" and "test" targets will not depend on "check" > target > "check" target will include checkstyle, rat and eclipse-warnings > > There is an additional flag "no-check" to disable checks in the "jar" target. > > Will not introduce any Git hook. > > wt., 27 cze 2023 o 18:35 Jacek Lewandowski > napisał(a): >> >> With git you can always opt-out by adding --no-verify flag to either push or >> commit >> >> I really prefer separate tasks than flags. Flags are not listed in the help >> message like "ant -p" and are not auto-completed in the terminal. That makes >> them almost undiscoverable for newcomers. >> >> Want to have jar include checks? Ok, but let's don't run checks >> automatically with "build" or "test" >> >> >> >> wt., 27 cze 2023 o 18:26 David Capwell napisał(a): >>> >>> nobody referred to running checks in a pre-push (or pre-commit) hook >>> >>> >>> >>> In accord I added an opt-out for each hook, and will require such here as >>> well… as long as you can opt-out, its fine by me… I know I will likely >>> opt-out, but wouldn’t block such an effort >>> >>> Your point that pre-push hook might not be the best one is valid, and we >>> should rather think about pre-commit >>> >>> >>> Pre-push is far better than pre-commit, with pre-commit you are forcing a >>> style on people…. I for one have many many commits in a single PR, where I >>> use commits to not loose track of progress (even if the code doesn’t >>> compile), so forcing the build to work would be a -1 from me…. Pre-push at >>> least means “you want the world to see this” so makes more sense there… >>> >>> Again, must have an opt-out >>> >>> proposed: >>> ant jar (just build) >>> git commit (run some checks) >>> >>> >>> I am against this, jar should also check and ask users to opt-out if they >>> don’t want the checks…. >>> >>> If we go with opt-out i'd like to see one flag that can disable all checks: >>> `-Dchecks.skip` >>> >>> >>> Works for me, you can also do the following to disable and not worry about >>> >>> $ cat < build.properties >>> checks.skip: true >>> EOF >>> >>> On Jun 27, 2023, at 3:14 AM, Mick Semb Wever wrote: >>> >>> The context is that we currently have 3 checks in the build: >>> >>> - Checkstyle, >>> - Eclipse-Warnings, >>> - RAT >>> >>> >>> >>> And dependency-check (owasp). >>> >>> >>> >>> I want to discuss whether you are ok with extracting all checks to their >>> distinct target and not running it automatically with the targets which >>> devs usually run locally. In particular: >>> >>> >>> "build", "jar", and all "test" targets would not trigger CheckStyle, RAT or >>> Eclipse-Warnings >>> A new target "check" would trigger all CheckStyle, RAT, and Eclipse-Warnings >>> The new "check" target would be run along with the "artifacts" target on >>> Jenkins-CI, and it as a separate build step in CircleCI >>> >>> >>> >>> +0 I prefer this opt-in over an opt-out approach. >>> >>> It should be separated from `artifacts` too. >>> We would need to encourage engineers to run `ant check` before >>> starting CI and/or requesting review. >>> >>> I'm in favour of the opt-in approach because it keeps it visible. >>> Folks configure flags and it "disappears" forever. Also it's a >>> headache in all the other ant targets where we actually don't want it, >>> e.g. tests. >>> >>> If we go with opt-out i'd like to see one flag that can disable all >>> checks: `-Dchecks.skip` >>> >>> >>> That could be fixed by running checks in a pre-push Git hook. There are >>> some benefits of this compared to the current behavior: >>> >>> >>> >>> -1 >>> committing and pushing to a personal branch is often done to save work >>> or for cross-machine or collaboration. We should not gate on checks or >>> compilation here. >>> >>> PRs should fail if checks fail, to give newcomers clear feedback (and >>> to take this nit-picking out of the review process). >>> >>>
Re: [DISCUSS] When to run CheckStyle and other verificiations
There is another target called "build", which retrieves dependencies, and then calls "build-project". czw., 29 cze 2023 o 12:33 Brandon Williams napisał(a): > This sounds good to me. Can we shorten 'build-project' to just 'build'? > > Kind Regards, > Brandon > > On Thu, Jun 29, 2023 at 3:22 AM Jacek Lewandowski > wrote: > > > > So given all the feedback, I'm going to do the following: > > > > "jar" will depend on "check" target > > "build-project", "build-test" and "test" targets will not depend on > "check" target > > "check" target will include checkstyle, rat and eclipse-warnings > > > > There is an additional flag "no-check" to disable checks in the "jar" > target. > > > > Will not introduce any Git hook. > > > > wt., 27 cze 2023 o 18:35 Jacek Lewandowski > napisał(a): > >> > >> With git you can always opt-out by adding --no-verify flag to either > push or commit > >> > >> I really prefer separate tasks than flags. Flags are not listed in the > help message like "ant -p" and are not auto-completed in the terminal. That > makes them almost undiscoverable for newcomers. > >> > >> Want to have jar include checks? Ok, but let's don't run checks > automatically with "build" or "test" > >> > >> > >> > >> wt., 27 cze 2023 o 18:26 David Capwell napisał(a): > >>> > >>> nobody referred to running checks in a pre-push (or pre-commit) hook > >>> > >>> > >>> > >>> In accord I added an opt-out for each hook, and will require such here > as well… as long as you can opt-out, its fine by me… I know I will likely > opt-out, but wouldn’t block such an effort > >>> > >>> Your point that pre-push hook might not be the best one is valid, and > we should rather think about pre-commit > >>> > >>> > >>> Pre-push is far better than pre-commit, with pre-commit you are > forcing a style on people…. I for one have many many commits in a single > PR, where I use commits to not loose track of progress (even if the code > doesn’t compile), so forcing the build to work would be a -1 from me…. > Pre-push at least means “you want the world to see this” so makes more > sense there… > >>> > >>> Again, must have an opt-out > >>> > >>> proposed: > >>> ant jar (just build) > >>> git commit (run some checks) > >>> > >>> > >>> I am against this, jar should also check and ask users to opt-out if > they don’t want the checks…. > >>> > >>> If we go with opt-out i'd like to see one flag that can disable all > checks: `-Dchecks.skip` > >>> > >>> > >>> Works for me, you can also do the following to disable and not worry > about > >>> > >>> $ cat < build.properties > >>> checks.skip: true > >>> EOF > >>> > >>> On Jun 27, 2023, at 3:14 AM, Mick Semb Wever wrote: > >>> > >>> The context is that we currently have 3 checks in the build: > >>> > >>> - Checkstyle, > >>> - Eclipse-Warnings, > >>> - RAT > >>> > >>> > >>> > >>> And dependency-check (owasp). > >>> > >>> > >>> > >>> I want to discuss whether you are ok with extracting all checks to > their distinct target and not running it automatically with the targets > which devs usually run locally. In particular: > >>> > >>> > >>> "build", "jar", and all "test" targets would not trigger CheckStyle, > RAT or Eclipse-Warnings > >>> A new target "check" would trigger all CheckStyle, RAT, and > Eclipse-Warnings > >>> The new "check" target would be run along with the "artifacts" target > on Jenkins-CI, and it as a separate build step in CircleCI > >>> > >>> > >>> > >>> +0 I prefer this opt-in over an opt-out approach. > >>> > >>> It should be separated from `artifacts` too. > >>> We would need to encourage engineers to run `ant check` before > >>> starting CI and/or requesting review. > >>> > >>> I'm in favour of the opt-in approach because it keeps it visible. > >>> Folks configure flags and it "disappears" forever. Also it's a > >>> headache in all the other ant targets where we actually don't want it, > >>> e.g. tests. > >>> > >>> If we go with opt-out i'd like to see one flag that can disable all > >>> checks: `-Dchecks.skip` > >>> > >>> > >>> That could be fixed by running checks in a pre-push Git hook. There > are some benefits of this compared to the current behavior: > >>> > >>> > >>> > >>> -1 > >>> committing and pushing to a personal branch is often done to save work > >>> or for cross-machine or collaboration. We should not gate on checks or > >>> compilation here. > >>> > >>> PRs should fail if checks fail, to give newcomers clear feedback (and > >>> to take this nit-picking out of the review process). > >>> > >>> >
Re: [DISCUSS] When to run CheckStyle and other verificiations
On Thu, 29 Jun 2023 at 13:30, Jacek Lewandowski wrote: > There is another target called "build", which retrieves dependencies, and > then calls "build-project". > Is it intended to be called by a user ? If not, please follow the ant style prefixing the target name with an underscore (so that it does not appear in the `ant -projecthelp` list). If possible, I agree with Brandon, `build` is the better name to expose to the user.
Re: [DISCUSS] When to run CheckStyle and other verificiations
> I really prefer separate tasks than flags. Flags are not listed in the help > message like "ant -p" and are not auto-completed in the terminal. That makes > them almost undiscoverable for newcomers. Please, no more flags. We are *more* than flaggy enough right now. Having to dig through build.xml to determine how to change things or do things is painful; the more we can avoid this (for oldtimers and newcomers alike!) the better. On Thu, Jun 29, 2023, at 8:34 AM, Mick Semb Wever wrote: > > > On Thu, 29 Jun 2023 at 13:30, Jacek Lewandowski > wrote: >> There is another target called "build", which retrieves dependencies, and >> then calls "build-project". > > > Is it intended to be called by a user ? > > If not, please follow the ant style prefixing the target name with an > underscore (so that it does not appear in the `ant -projecthelp` list). > > If possible, I agree with Brandon, `build` is the better name to expose to > the user.
Re: Improved DeletionTime serialization to reduce disk size
> I would prefer we not plan on two distinct changes to this I agree with this sentiment, **and** > +1, if you have time for this approach and no other in this window. People are going to use 5.0 for awhile. Better to have an improvement in their hands for that duration than no improvement at all IMO. Justifies the cost of the double implementation and transitions to me. On Tue, Jun 27, 2023, at 5:43 AM, Mick Semb Wever wrote: >> Just for completeness the change is a handful loc. The rest is added tests >> and we'd loose the sstable format change opportunity window. >> > > > +1, if you have time for this approach and no other in this window. > > (If you have time for the other, or someone else does, then the technically > superior approach should win) > > >
Re: CASSANDRA-18554 - mTLS based client and internode authenticators
I like the idea of extending CREATE ROLE rather than adding a brand new ADD
IDENTITY syntax. Not sure how that can line up with one to many
relationships for an identity, but maybe that can just be done through role
hierarchy?
In either case, I don’t think IDENTITY related operations should be tied to
the super user flag. They should be tied to either existing role
permissions, or a brand new permissions about IDENTITY. We should not
require that end users give the account allowed to make IDENTITY changes
super user permission to do what ever they want across the whole database.
On Jun 28, 2023 at 11:48:02 PM, Yuki Morishita wrote:
> Thinking more about "CREATE ROLE" permission, if we can extend CREATE
> ROLE/ALTER ROLE statements, it may look streamlined:
>
> I don't have the good example, but something like:
> ```
> CREATE ROLE dev WITH LOGIN = true AND IDENTITIES = {'spiffe://xxx'};
> ALTER ROLE dev ADD IDENTITY 'xxx';
> LIST ROLES;
> ```
>
> This requires a role to identities table as well as the current identity
> to role table though.
>
> On Thu, Jun 29, 2023 at 12:34 PM Yuki Morishita
> wrote:
>
>> Hi Jyothsna,
>>
>> I think for the *initial* commit, the description looks fine to me.
>> I'd like to see/contribute to the future improvement though:
>>
>> * ADD IDENTITY requires SUPERUSER, this means that the brand new cluster
>> needs to start with PasswordAuthenticator/CassandraAuthorizer first, and
>> then change to mTLS one.
>> * For this, I'd really like to see Cassandra use password authn and
>> authz by default.
>> * Cassandra allows the user with "CREATE ROLE" permission to create
>> roles without superuser privilege. Maybe it is natural to allow them to add
>> identities also?
>>
>>
>> On Thu, Jun 29, 2023 at 7:35 AM Jyothsna Konisa
>> wrote:
>>
>>> Hi Yuki,
>>>
>>> I have added cassandra docs for CQL syntax that we are adding and how to
>>> get started with using mTLS authenticators along with the migration plan.
>>> Please review it and let me know if it looks good.
>>>
>>> Thanks,
>>> Jyothsna Konisa.
>>>
>>> On Wed, Jun 21, 2023 at 10:46 AM Jyothsna Konisa
>>> wrote:
>>>
Hi Yuki!
Thanks for the questions.
Here are the steps for the initial setup.
1. Since only super users can add/remove identities from the
`identity_to_roles` table, operators should use that role to add authorized
identities to the table. Note that the authenticator is not an mTLS
authenticator yet.
EX: ADD IDENTITY 'spiffe://testdomain.com/testIdentifier/testValue
'
TO ROLE 'read_only_user'
2. Change authenticator configuration in cassandra.yaml to use mTLS
authenticator
EX: authenticator:
class_name :org.apache.cassandra.auth.MutualTlsAuthenticator
parameters :
validator_class_name:
org.apache.cassandra.auth.SpiffeCertificateValidator
3. Restart the cluster so that newly configured mTLS authenticator is
used
What will be the op's first step to set up the roles and identities?
-> Yes, the op should set up roles & identities first.
Is default cassandra / cassandra superuser login still required to set
up other roles and identities?
-> When transitioning from a password based to mTLS based
authenticators, yes superuser login is required to add identities, as only
super users can add them. However when a cluster is using mTLS based
authenticator, the super user will be associated with some certificate
identity and hence we don't need password based cassandra super user login.
If initial cassandra super user login is required, does that mean super
users and "cassandra '' superuser bypass mTLS check?
-> No, while adding identities to the roles table in step1 the
authenticator will not be an mTLS authenticator. Once the identities are
added and the authenticator is configured, even super users have to go
through an mTLS check during connection.
Regarding migration
I *think* you need to first use
MutualTlsWithPasswordFallbackAuthenticator so the current roles can login
with their password,
and eventually the admin sets up identity and then can switch to mTLS
auth.
Is this the expected way for migration?
-> Yes you can do that or else we can add identities with password
based login and then change the authenticator to be mTLS authenticator.
I think a thorough documentation for this new feature including new CQL
syntax, setting up and migration would be greatly appreciated.
-> I have added documentation for the authenticators, cqlsh commands in
the Javadocs in the source code. Maybe I will add the setup process &
migration process in the Javadocs, d
Re: Improved DeletionTime serialization to reduce disk size
So I’m just taking a quick peek at SerializationHeader and we already have a method for reading and writing a deletion time with offsets from EncodingStats. So perhaps we simply have a bug where we are using DeletionTime Serializer instead of SerializationHeader.writeLocalDeletionTime? It looks to me like this is already available at most (perhaps all) of the relevant call sites. > On 29 Jun 2023, at 15:53, Josh McKenzie wrote: > > >> >> I would prefer we not plan on two distinct changes to this > I agree with this sentiment, and > >> +1, if you have time for this approach and no other in this window. > People are going to use 5.0 for awhile. Better to have an improvement in > their hands for that duration than no improvement at all IMO. Justifies the > cost of the double implementation and transitions to me. > >> On Tue, Jun 27, 2023, at 5:43 AM, Mick Semb Wever wrote: >> Just for completeness the change is a handful loc. The rest is added tests >> and we'd loose the sstable format change opportunity window. >> >> >> >> +1, if you have time for this approach and no other in this window. >> >> (If you have time for the other, or someone else does, then the technically >> superior approach should win) >> >> >
Re: Improved DeletionTime serialization to reduce disk size
On Thu, Jun 22, 2023 at 11:23 PM Berenguer Blasi wrote: > Hi all, > > Given we're already introducing a new sstable format (OA) in 5.0 I would > like to try to get this in before the freeze. The point being that > sstables with lots of small partitions would benefit from a smaller DT > at partition level. My tests show a 3%-4% size reduction on disk. > 3-4% reduction on disk ... for what exactly? It seems exceptionally uncommon to have 3% of your data SIZE be tombstones. Is this enhancement driven by a pathological data model that's like "mostly tiny records OR tombstones" ?
Re: Improved DeletionTime serialization to reduce disk size
On Thu, Jun 29, 2023 at 11:42 AM Jeff Jirsa wrote: > 3-4% reduction on disk ... for what exactly? > > It seems exceptionally uncommon to have 3% of your data SIZE be tombstones. If the data is TTL'd I think it's not entirely uncommon. Kind Regards, Brandon
Re: [DISCUSS] When to run CheckStyle and other verificiations
Should we just keep a consolidated for all kind of checks no-check flag and get rid of the no-checkstyle one? Trading one for one with Josh :-) Best regards, Ekaterina On Thu, 29 Jun 2023 at 10:52, Josh McKenzie wrote: > I really prefer separate tasks than flags. Flags are not listed in the > help message like "ant -p" and are not auto-completed in the terminal. That > makes them almost undiscoverable for newcomers. > > Please, no more flags. We are *more* than flaggy enough right now. > > Having to dig through build.xml to determine how to change things or do > things is painful; the more we can avoid this (for oldtimers and newcomers > alike!) the better. > > On Thu, Jun 29, 2023, at 8:34 AM, Mick Semb Wever wrote: > > > > On Thu, 29 Jun 2023 at 13:30, Jacek Lewandowski < > lewandowski.ja...@gmail.com> wrote: > > There is another target called "build", which retrieves dependencies, and > then calls "build-project". > > > > Is it intended to be called by a user ? > > If not, please follow the ant style prefixing the target name with an > underscore (so that it does not appear in the `ant -projecthelp` list). > > If possible, I agree with Brandon, `build` is the better name to expose to > the user. > > >
Re: [DISCUSS] When to run CheckStyle and other verificiations
> In accord I added an opt-out for each hook, and will require such here as well On for main branches, off for feature branches seems like it might blanket satisfy this concern? Doesn't fix the "--atomic across 5 branches means style checks and build on hook across those branches" which isn't ideal. I don't think style check failures after push upstream are frequent enough to make the cost/benefit there make sense overall are they? Related to this - I have sonarlint, spotbugs, and checkstyle all running inside idea; since pulling those in and tuning the configs a bit I haven't run into a single issue w/our checkstyle build target (go figure). Having the required style checks reflected realtime inside your work environment goes a long way towards making it a more intuitive part of your workflow rather than being an annoying last minute block of your ability to progress that requires circling back into the code. >From a technical perspective, it looks like adding a reference >"externalDependencies.xml" to our ide/idea directory which we copied over >during "generate-idea-files" would be sufficient to get idea to pop up prompts >to install those extensions if you don't have them when opening the project >(theory; haven't tested). We'd need to make sure the configuration for each of those was calibrated to our project out of the box of course, but making style considerations a first-class citizen in that way seems a more intuitive and human-centered approach to all this rather than debating nuance of our command-line targets, hooks, and how we present things to people. To Berenguer's point - better to have these be completely invisible to people with their workflows and Just Work (except for when your IDE scolds you for bad behavior w/build errors immediately). I still think Flags Are Bad. :) On Thu, Jun 29, 2023, at 1:38 PM, Ekaterina Dimitrova wrote: > Should we just keep a consolidated for all kind of checks no-check flag and > get rid of the no-checkstyle one? > > Trading one for one with Josh :-) > > Best regards, > Ekaterina > > On Thu, 29 Jun 2023 at 10:52, Josh McKenzie wrote: >> __ >>> I really prefer separate tasks than flags. Flags are not listed in the help >>> message like "ant -p" and are not auto-completed in the terminal. That >>> makes them almost undiscoverable for newcomers. >> Please, no more flags. We are *more* than flaggy enough right now. >> >> Having to dig through build.xml to determine how to change things or do >> things is painful; the more we can avoid this (for oldtimers and newcomers >> alike!) the better. >> >> On Thu, Jun 29, 2023, at 8:34 AM, Mick Semb Wever wrote: >>> >>> >>> On Thu, 29 Jun 2023 at 13:30, Jacek Lewandowski >>> wrote: There is another target called "build", which retrieves dependencies, and then calls "build-project". >>> >>> >>> Is it intended to be called by a user ? >>> >>> If not, please follow the ant style prefixing the target name with an >>> underscore (so that it does not appear in the `ant -projecthelp` list). >>> >>> If possible, I agree with Brandon, `build` is the better name to expose to >>> the user. >>
Re: [DISCUSS] When to run CheckStyle and other verificiations
+100 I support making generate-idea-files auto setup everything in IntelliJ for you. If you post a diff, I will test it. On this proposal, I don’t really have an opinion one way or the other about what the default is for local "ant jar”, if its slow I will figure out how to turn it off, if its fast I will leave it on. I do care that CI runs checks, and complains loudly if something is wrong such that it is very easy to tell during review. -Jeremiah On Jun 29, 2023 at 1:44:09 PM, Josh McKenzie wrote: > In accord I added an opt-out for each hook, and will require such here as > well > > On for main branches, off for feature branches seems like it might blanket > satisfy this concern? Doesn't fix the "--atomic across 5 branches means > style checks and build on hook across those branches" which isn't ideal. I > don't think style check failures after push upstream are frequent enough to > make the cost/benefit there make sense overall are they? > > Related to this - I have sonarlint, spotbugs, and checkstyle all running > inside idea; since pulling those in and tuning the configs a bit I haven't > run into a single issue w/our checkstyle build target (go figure). Having > the required style checks reflected realtime inside your work environment > goes a long way towards making it a more intuitive part of your workflow > rather than being an annoying last minute block of your ability to progress > that requires circling back into the code. > > From a technical perspective, it looks like adding a reference > "externalDependencies.xml" to our ide/idea directory which we copied over > during "generate-idea-files" would be sufficient to get idea to pop up > prompts to install those extensions if you don't have them when opening the > project (theory; haven't tested). > > We'd need to make sure the configuration for each of those was calibrated > to our project out of the box of course, but making style considerations a > first-class citizen in that way seems a more intuitive and human-centered > approach to all this rather than debating nuance of our command-line > targets, hooks, and how we present things to people. To Berenguer's point - > better to have these be completely invisible to people with their workflows > and Just Work (except for when your IDE scolds you for bad behavior w/build > errors immediately). > > I still think Flags Are Bad. :) > > On Thu, Jun 29, 2023, at 1:38 PM, Ekaterina Dimitrova wrote: > > Should we just keep a consolidated for all kind of checks no-check flag > and get rid of the no-checkstyle one? > > Trading one for one with Josh :-) > > Best regards, > Ekaterina > > On Thu, 29 Jun 2023 at 10:52, Josh McKenzie wrote: > > > I really prefer separate tasks than flags. Flags are not listed in the > help message like "ant -p" and are not auto-completed in the terminal. That > makes them almost undiscoverable for newcomers. > > Please, no more flags. We are *more* than flaggy enough right now. > > Having to dig through build.xml to determine how to change things or do > things is painful; the more we can avoid this (for oldtimers and newcomers > alike!) the better. > > On Thu, Jun 29, 2023, at 8:34 AM, Mick Semb Wever wrote: > > > > On Thu, 29 Jun 2023 at 13:30, Jacek Lewandowski < > lewandowski.ja...@gmail.com> wrote: > > There is another target called "build", which retrieves dependencies, and > then calls "build-project". > > > > Is it intended to be called by a user ? > > If not, please follow the ant style prefixing the target name with an > underscore (so that it does not appear in the `ant -projecthelp` list). > > If possible, I agree with Brandon, `build` is the better name to expose to > the user. > > > >
Re: [DISCUSS] When to run CheckStyle and other verificiations
Great stuff, Josh! This is what I was talking about when I was mentioning that I am super curious about the workflows of other people. Any chance you share your setup somewhere so I may try it? Too soon to tell if we indeed want to go that direction but trying it out would be great From: Josh McKenzie Sent: Thursday, June 29, 2023 20:44 To: dev Subject: Re: [DISCUSS] When to run CheckStyle and other verificiations NetApp Security WARNING: This is an external email. Do not click links or open attachments unless you recognize the sender and know the content is safe. In accord I added an opt-out for each hook, and will require such here as well On for main branches, off for feature branches seems like it might blanket satisfy this concern? Doesn't fix the "--atomic across 5 branches means style checks and build on hook across those branches" which isn't ideal. I don't think style check failures after push upstream are frequent enough to make the cost/benefit there make sense overall are they? Related to this - I have sonarlint, spotbugs, and checkstyle all running inside idea; since pulling those in and tuning the configs a bit I haven't run into a single issue w/our checkstyle build target (go figure). Having the required style checks reflected realtime inside your work environment goes a long way towards making it a more intuitive part of your workflow rather than being an annoying last minute block of your ability to progress that requires circling back into the code. >From a technical perspective, it looks like adding a reference >"externalDependencies.xml" to our ide/idea directory which we copied over >during "generate-idea-files" would be sufficient to get idea to pop up prompts >to install those extensions if you don't have them when opening the project >(theory; haven't tested). We'd need to make sure the configuration for each of those was calibrated to our project out of the box of course, but making style considerations a first-class citizen in that way seems a more intuitive and human-centered approach to all this rather than debating nuance of our command-line targets, hooks, and how we present things to people. To Berenguer's point - better to have these be completely invisible to people with their workflows and Just Work (except for when your IDE scolds you for bad behavior w/build errors immediately). I still think Flags Are Bad. :) On Thu, Jun 29, 2023, at 1:38 PM, Ekaterina Dimitrova wrote: Should we just keep a consolidated for all kind of checks no-check flag and get rid of the no-checkstyle one? Trading one for one with Josh :-) Best regards, Ekaterina On Thu, 29 Jun 2023 at 10:52, Josh McKenzie mailto:jmcken...@apache.org>> wrote: I really prefer separate tasks than flags. Flags are not listed in the help message like "ant -p" and are not auto-completed in the terminal. That makes them almost undiscoverable for newcomers. Please, no more flags. We are more than flaggy enough right now. Having to dig through build.xml to determine how to change things or do things is painful; the more we can avoid this (for oldtimers and newcomers alike!) the better. On Thu, Jun 29, 2023, at 8:34 AM, Mick Semb Wever wrote: On Thu, 29 Jun 2023 at 13:30, Jacek Lewandowski mailto:lewandowski.ja...@gmail.com>> wrote: There is another target called "build", which retrieves dependencies, and then calls "build-project". Is it intended to be called by a user ? If not, please follow the ant style prefixing the target name with an underscore (so that it does not appear in the `ant -projecthelp` list). If possible, I agree with Brandon, `build` is the better name to expose to the user.
Re: [DISCUSS] When to run CheckStyle and other verificiations
There is a separate thread started and respective ticket for generate-idea-files. https://lists.apache.org/thread/o2fdkyv2skvf9ngy9jhpnhvo92qvr17m CASSANDRA-18467 On Thu, 29 Jun 2023 at 16:54, Jeremiah Jordan wrote: > +100 I support making generate-idea-files auto setup everything in > IntelliJ for you. If you post a diff, I will test it. > > On this proposal, I don’t really have an opinion one way or the other > about what the default is for local "ant jar”, if its slow I will figure > out how to turn it off, if its fast I will leave it on. > I do care that CI runs checks, and complains loudly if something is wrong > such that it is very easy to tell during review. > > -Jeremiah > > On Jun 29, 2023 at 1:44:09 PM, Josh McKenzie wrote: > >> In accord I added an opt-out for each hook, and will require such here as >> well >> >> On for main branches, off for feature branches seems like it might >> blanket satisfy this concern? Doesn't fix the "--atomic across 5 branches >> means style checks and build on hook across those branches" which isn't >> ideal. I don't think style check failures after push upstream are frequent >> enough to make the cost/benefit there make sense overall are they? >> >> Related to this - I have sonarlint, spotbugs, and checkstyle all running >> inside idea; since pulling those in and tuning the configs a bit I haven't >> run into a single issue w/our checkstyle build target (go figure). Having >> the required style checks reflected realtime inside your work environment >> goes a long way towards making it a more intuitive part of your workflow >> rather than being an annoying last minute block of your ability to progress >> that requires circling back into the code. >> >> From a technical perspective, it looks like adding a reference >> "externalDependencies.xml" to our ide/idea directory which we copied over >> during "generate-idea-files" would be sufficient to get idea to pop up >> prompts to install those extensions if you don't have them when opening the >> project (theory; haven't tested). >> >> We'd need to make sure the configuration for each of those was calibrated >> to our project out of the box of course, but making style considerations a >> first-class citizen in that way seems a more intuitive and human-centered >> approach to all this rather than debating nuance of our command-line >> targets, hooks, and how we present things to people. To Berenguer's point - >> better to have these be completely invisible to people with their workflows >> and Just Work (except for when your IDE scolds you for bad behavior w/build >> errors immediately). >> >> I still think Flags Are Bad. :) >> >> On Thu, Jun 29, 2023, at 1:38 PM, Ekaterina Dimitrova wrote: >> >> Should we just keep a consolidated for all kind of checks no-check flag >> and get rid of the no-checkstyle one? >> >> Trading one for one with Josh :-) >> >> Best regards, >> Ekaterina >> >> On Thu, 29 Jun 2023 at 10:52, Josh McKenzie wrote: >> >> >> I really prefer separate tasks than flags. Flags are not listed in the >> help message like "ant -p" and are not auto-completed in the terminal. That >> makes them almost undiscoverable for newcomers. >> >> Please, no more flags. We are *more* than flaggy enough right now. >> >> Having to dig through build.xml to determine how to change things or do >> things is painful; the more we can avoid this (for oldtimers and newcomers >> alike!) the better. >> >> On Thu, Jun 29, 2023, at 8:34 AM, Mick Semb Wever wrote: >> >> >> >> On Thu, 29 Jun 2023 at 13:30, Jacek Lewandowski < >> lewandowski.ja...@gmail.com> wrote: >> >> There is another target called "build", which retrieves dependencies, and >> then calls "build-project". >> >> >> >> Is it intended to be called by a user ? >> >> If not, please follow the ant style prefixing the target name with an >> underscore (so that it does not appear in the `ant -projecthelp` list). >> >> If possible, I agree with Brandon, `build` is the better name to expose >> to the user. >> >> >> >>
