Hi devs, I was wondering... How good is your "good enough"? It's a question I often ask myself, when writing patches or QAing yours: is it good enough to be into Koha? It does not have to be perfect or it may never reach master, but it must meet certain standards. When I was RM I tried to put that limit quite high. Not necessarily by asking the author for improving the follow-up patches, but also by adding the missing bits myself. There are various types of "good enough", depending on what we look at: good enough to be reviewed, good enough to be tested, to be put in production, get feedback, try an idea, etc. Most of the time, what is driving the limit is answering the questions "Is it maintainable?" / "Is it future proof?". Making sure the code you are writing won't be broken inadvertently is very important and must be part of the reflection. Katrin asked the QA team members what were our plans for 23.05. In my opinion we should enforce this "be future proof" concept. Writing code is easy, especially in Koha (yes it is!). Writing maintainable and robust code, following our guidelines is a bit harder. That's why we have a QA process that tries to catch inconsistencies or edge cases you may have missed. But I think we can be even more rigorous, and aim for better standards. We can elevate our ambitions and do better. When we see the number of new enhancements we are releasing every 6 months, it shows well that writing code is definitely not a problem. However sometimes developers are tempted to abandon their work once they are pushed to master. Pushed does not mean "done". Providing bug fixes, following-up with patches when needed, fixing CI jenkins, etc. is part of job (/fun) As a Koha developer for a long time now, I know how frustrating it can be to be asked for follow-ups/rewrite/tests to have our patches stamped with the precious PQA mark. But from the other point of view (RM, RMaints, QA team), I also know it's very frustrating when you are in charge of the release and you do not get the appropriate follow-up work once it's pushed to master. There are some easy steps to write/review better patches. All have been discussed already several times, but that can be enforced even more: 1. Perltidy (!) This is really a very trivial step. Please perltidy your code. There are hundreds of commits that have been pushed in the last months that are not tidy (alignment, indentation, lines too long, etc.) This can easily be configured in your IDE! [1] 2. Provide clean code. As said it's not necessarily easy, but the QA team and RM are supposed to know if the code is clean regarding Koha guidelines. If the code is not clean, don't PQA, don't push. Either clean yourself, or ask the original author of the patch to do it (explaining to them how it can be improved ofc). 3. Squash! I have been away for a couple of months and had to read the git history to know what I missed. And it was really hard to follow what was going on. First of all, we are not consistent: the commit message must tell what the patch is doing, not what the bug was (if you are writing a bug fix). Then, there are way too many follow-ups: tidiness, indentation fix, typo, spelling, etc. All those tiny follow-ups could be squashed into the original patch. We don't need unnecessary tons of entries in our git log for that. For instance, I usually add a "JD Amended patch: perltidy" for instance when I tidy the original patch, to keep track of the modification. Squash can be done by the original author, the QAer, the RM. So yes, you are losing one commit in the stats but the git log is easy to read! We could have an "Amended-by" marker if we really want to add credit on the dashboard (and/or release notes). 4. Run tests. Don't wait for Jenkins to fail. This is valid for the author and QA. Anticipate the failures by running more tests. If you are modifying C4::Circulation, then run prove on t/db_dependent/Circulation*, not only Circulation.t. It will help you catch edge cases. When something is pushed, track down jenkins failures that could be caused by your patches. 6. Be strict if you are QAing. Each QA member has their own "good enough", and the RM as well (either relying on the QAer or providing a full review). But QA must fail if the code is old Koha style code, or not "good enough". 7. Provide support for failing tests, fix things you broke. The QA team will be more comfortable with your patches if you show them you are providing support for your stuff. It's not because it's pushed that you don't have any more efforts to make. Provide follow-up patches you promised, provide bug fixes, etc. We don't have a good way to keep track of such demands, which does not make tracking easier for devs, QA and RM. Any suggestions? 8. QA team MUST NEVER* pass QA a change that is not covered by tests, never. You should not provide change to modules without tests! * almost never... 9. Stick to existing patterns. We should not have different ways to do the same thing. We should not have different places where a code is doing the same thing. Ask for help or advice on the list or IRC before you start coding. We will be happy to guide you. Even if you are a regular Koha developer it's not always easy to be aware of the latest master changes. We will tell you what's the current good practice, or point you to examples you could reuse for what you want to implement. 10. CI should drive the pushes. No more push if CI is not green. The more we wait the harder it is to track down the origin of the problem. Last cycle some jobs have been red for months, and we released 22.11.00 with D10, D11, D12 marked unstable... What will I do next cycle? All of that, and more. I will track down jenkins failures and responsibilize developers telling them when they break tests (and won't fix them anymore as I have been doing for years). I will raise on the bug reports what could have been improved. Yes, read that I will be even more annoying (to put it politely) than before. I've noticed that the pre-commit git hook on the wiki has been broken for more than 3 years. And also caught some core developers that do not have it in place. I am relying on it to keep Vue files tidy so it's important to have it set up properly. I am planning to force its usage for ktd users [2]. Adding more checks to it will help us to catch inconsistencies from the beginning. To summarize, writing code is cheap, maintaining code is way more expensive! It is easier to get the attention of developers before the patches are pushed to master than after, so we could be more ambitious and ask more. For discussion :) Cheers, Jonathan [1] If you are using vim, open ~/vimrc, add vmap <F8> :!perltidy -q<CR> Reload vim, select code in visual mode [2] https://gitlab.com/koha-community/koha-misc4dev/-/issues/59
Hi, It seems to me that several issues could be solved by having the CI run sooner, so authors can have feedback as soon as possible. Something like github's CI integration with pull requests would be amazing to detect common mistakes early and stop wasting human time. We should know if tests pass before pushing to master, not after. We could check tidyness, commit message conventions, code coverage by tests, ... all before another person have to look at it. Also tests are not easy to run locally. They might pass on Jenkins, but they do not on my local setup, so they are basically useless to tell me if I broke something. It also makes writing new tests more difficult than it should. If I wanted to improve Koha developer experience, I would start with that. My 2 cents. Le 02/12/2022 à 15:42, Jonathan Druart a écrit :
Hi devs,
I was wondering... How good is your "good enough"? It's a question I often ask myself, when writing patches or QAing yours: is it good enough to be into Koha? It does not have to be perfect or it may never reach master, but it must meet certain standards.
When I was RM I tried to put that limit quite high. Not necessarily by asking the author for improving the follow-up patches, but also by adding the missing bits myself.
There are various types of "good enough", depending on what we look at: good enough to be reviewed, good enough to be tested, to be put in production, get feedback, try an idea, etc.
Most of the time, what is driving the limit is answering the questions "Is it maintainable?" / "Is it future proof?". Making sure the code you are writing won't be broken inadvertently is very important and must be part of the reflection.
Katrin asked the QA team members what were our plans for 23.05. In my opinion we should enforce this "be future proof" concept. Writing code is easy, especially in Koha (yes it is!). Writing maintainable and robust code, following our guidelines is a bit harder. That's why we have a QA process that tries to catch inconsistencies or edge cases you may have missed. But I think we can be even more rigorous, and aim for better standards. We can elevate our ambitions and do better. When we see the number of new enhancements we are releasing every 6 months, it shows well that writing code is definitely not a problem. However sometimes developers are tempted to abandon their work once they are pushed to master. Pushed does not mean "done". Providing bug fixes, following-up with patches when needed, fixing CI jenkins, etc. is part of job (/fun)
As a Koha developer for a long time now, I know how frustrating it can be to be asked for follow-ups/rewrite/tests to have our patches stamped with the precious PQA mark. But from the other point of view (RM, RMaints, QA team), I also know it's very frustrating when you are in charge of the release and you do not get the appropriate follow-up work once it's pushed to master.
There are some easy steps to write/review better patches. All have been discussed already several times, but that can be enforced even more: 1. Perltidy (!) This is really a very trivial step. Please perltidy your code. There are hundreds of commits that have been pushed in the last months that are not tidy (alignment, indentation, lines too long, etc.) This can easily be configured in your IDE! [1]
2. Provide clean code. As said it's not necessarily easy, but the QA team and RM are supposed to know if the code is clean regarding Koha guidelines. If the code is not clean, don't PQA, don't push. Either clean yourself, or ask the original author of the patch to do it (explaining to them how it can be improved ofc).
3. Squash! I have been away for a couple of months and had to read the git history to know what I missed. And it was really hard to follow what was going on. First of all, we are not consistent: the commit message must tell what the patch is doing, not what the bug was (if you are writing a bug fix). Then, there are way too many follow-ups: tidiness, indentation fix, typo, spelling, etc. All those tiny follow-ups could be squashed into the original patch. We don't need unnecessary tons of entries in our git log for that. For instance, I usually add a "JD Amended patch: perltidy" for instance when I tidy the original patch, to keep track of the modification. Squash can be done by the original author, the QAer, the RM. So yes, you are losing one commit in the stats but the git log is easy to read! We could have an "Amended-by" marker if we really want to add credit on the dashboard (and/or release notes).
4. Run tests. Don't wait for Jenkins to fail. This is valid for the author and QA. Anticipate the failures by running more tests. If you are modifying C4::Circulation, then run prove on t/db_dependent/Circulation*, not only Circulation.t. It will help you catch edge cases. When something is pushed, track down jenkins failures that could be caused by your patches.
6. Be strict if you are QAing. Each QA member has their own "good enough", and the RM as well (either relying on the QAer or providing a full review). But QA must fail if the code is old Koha style code, or not "good enough".
7. Provide support for failing tests, fix things you broke. The QA team will be more comfortable with your patches if you show them you are providing support for your stuff. It's not because it's pushed that you don't have any more efforts to make. Provide follow-up patches you promised, provide bug fixes, etc. We don't have a good way to keep track of such demands, which does not make tracking easier for devs, QA and RM. Any suggestions?
8. QA team MUST NEVER* pass QA a change that is not covered by tests, never. You should not provide change to modules without tests! * almost never...
9. Stick to existing patterns. We should not have different ways to do the same thing. We should not have different places where a code is doing the same thing. Ask for help or advice on the list or IRC before you start coding. We will be happy to guide you. Even if you are a regular Koha developer it's not always easy to be aware of the latest master changes. We will tell you what's the current good practice, or point you to examples you could reuse for what you want to implement.
10. CI should drive the pushes. No more push if CI is not green. The more we wait the harder it is to track down the origin of the problem. Last cycle some jobs have been red for months, and we released 22.11.00 with D10, D11, D12 marked unstable...
What will I do next cycle? All of that, and more. I will track down jenkins failures and responsibilize developers telling them when they break tests (and won't fix them anymore as I have been doing for years). I will raise on the bug reports what could have been improved. Yes, read that I will be even more annoying (to put it politely) than before.
I've noticed that the pre-commit git hook on the wiki has been broken for more than 3 years. And also caught some core developers that do not have it in place. I am relying on it to keep Vue files tidy so it's important to have it set up properly. I am planning to force its usage for ktd users [2]. Adding more checks to it will help us to catch inconsistencies from the beginning.
To summarize, writing code is cheap, maintaining code is way more expensive! It is easier to get the attention of developers before the patches are pushed to master than after, so we could be more ambitious and ask more.
For discussion :)
Cheers, Jonathan
[1] If you are using vim, open ~/vimrc, add vmap <F8> :!perltidy -q<CR> Reload vim, select code in visual mode [2] https://gitlab.com/koha-community/koha-misc4dev/-/issues/59 _______________________________________________ Koha-devel mailing list Koha-devel@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-devel website : https://www.koha-community.org/ git : https://git.koha-community.org/ bugs : https://bugs.koha-community.org/
-- Julian Maurice BibLibre
I don't think we should run the whole test suite everytime we attach patches, that would be very expensive in terms of resources. However it would be interesting to have a temporary 'master' branch that would become 'master' only if jenkins is happy. "master" would never be broken :D Using koha-testing-docker it's very easy to run tests locally. It will be even more easier with https://gitlab.com/koha-community/koha-misc4dev/-/issues/58 Le ven. 2 déc. 2022 à 18:16, Julian Maurice <julian.maurice@biblibre.com> a écrit :
Hi,
It seems to me that several issues could be solved by having the CI run sooner, so authors can have feedback as soon as possible. Something like github's CI integration with pull requests would be amazing to detect common mistakes early and stop wasting human time. We should know if tests pass before pushing to master, not after. We could check tidyness, commit message conventions, code coverage by tests, ... all before another person have to look at it.
Also tests are not easy to run locally. They might pass on Jenkins, but they do not on my local setup, so they are basically useless to tell me if I broke something. It also makes writing new tests more difficult than it should.
If I wanted to improve Koha developer experience, I would start with that.
My 2 cents.
Le 02/12/2022 à 15:42, Jonathan Druart a écrit :
Hi devs,
I was wondering... How good is your "good enough"? It's a question I often ask myself, when writing patches or QAing yours: is it good enough to be into Koha? It does not have to be perfect or it may never reach master, but it must meet certain standards.
When I was RM I tried to put that limit quite high. Not necessarily by asking the author for improving the follow-up patches, but also by adding the missing bits myself.
There are various types of "good enough", depending on what we look at: good enough to be reviewed, good enough to be tested, to be put in production, get feedback, try an idea, etc.
Most of the time, what is driving the limit is answering the questions "Is it maintainable?" / "Is it future proof?". Making sure the code you are writing won't be broken inadvertently is very important and must be part of the reflection.
Katrin asked the QA team members what were our plans for 23.05. In my opinion we should enforce this "be future proof" concept. Writing code is easy, especially in Koha (yes it is!). Writing maintainable and robust code, following our guidelines is a bit harder. That's why we have a QA process that tries to catch inconsistencies or edge cases you may have missed. But I think we can be even more rigorous, and aim for better standards. We can elevate our ambitions and do better. When we see the number of new enhancements we are releasing every 6 months, it shows well that writing code is definitely not a problem. However sometimes developers are tempted to abandon their work once they are pushed to master. Pushed does not mean "done". Providing bug fixes, following-up with patches when needed, fixing CI jenkins, etc. is part of job (/fun)
As a Koha developer for a long time now, I know how frustrating it can be to be asked for follow-ups/rewrite/tests to have our patches stamped with the precious PQA mark. But from the other point of view (RM, RMaints, QA team), I also know it's very frustrating when you are in charge of the release and you do not get the appropriate follow-up work once it's pushed to master.
There are some easy steps to write/review better patches. All have been discussed already several times, but that can be enforced even more: 1. Perltidy (!) This is really a very trivial step. Please perltidy your code. There are hundreds of commits that have been pushed in the last months that are not tidy (alignment, indentation, lines too long, etc.) This can easily be configured in your IDE! [1]
2. Provide clean code. As said it's not necessarily easy, but the QA team and RM are supposed to know if the code is clean regarding Koha guidelines. If the code is not clean, don't PQA, don't push. Either clean yourself, or ask the original author of the patch to do it (explaining to them how it can be improved ofc).
3. Squash! I have been away for a couple of months and had to read the git history to know what I missed. And it was really hard to follow what was going on. First of all, we are not consistent: the commit message must tell what the patch is doing, not what the bug was (if you are writing a bug fix). Then, there are way too many follow-ups: tidiness, indentation fix, typo, spelling, etc. All those tiny follow-ups could be squashed into the original patch. We don't need unnecessary tons of entries in our git log for that. For instance, I usually add a "JD Amended patch: perltidy" for instance when I tidy the original patch, to keep track of the modification. Squash can be done by the original author, the QAer, the RM. So yes, you are losing one commit in the stats but the git log is easy to read! We could have an "Amended-by" marker if we really want to add credit on the dashboard (and/or release notes).
4. Run tests. Don't wait for Jenkins to fail. This is valid for the author and QA. Anticipate the failures by running more tests. If you are modifying C4::Circulation, then run prove on t/db_dependent/Circulation*, not only Circulation.t. It will help you catch edge cases. When something is pushed, track down jenkins failures that could be caused by your patches.
6. Be strict if you are QAing. Each QA member has their own "good enough", and the RM as well (either relying on the QAer or providing a full review). But QA must fail if the code is old Koha style code, or not "good enough".
7. Provide support for failing tests, fix things you broke. The QA team will be more comfortable with your patches if you show them you are providing support for your stuff. It's not because it's pushed that you don't have any more efforts to make. Provide follow-up patches you promised, provide bug fixes, etc. We don't have a good way to keep track of such demands, which does not make tracking easier for devs, QA and RM. Any suggestions?
8. QA team MUST NEVER* pass QA a change that is not covered by tests, never. You should not provide change to modules without tests! * almost never...
9. Stick to existing patterns. We should not have different ways to do the same thing. We should not have different places where a code is doing the same thing. Ask for help or advice on the list or IRC before you start coding. We will be happy to guide you. Even if you are a regular Koha developer it's not always easy to be aware of the latest master changes. We will tell you what's the current good practice, or point you to examples you could reuse for what you want to implement.
10. CI should drive the pushes. No more push if CI is not green. The more we wait the harder it is to track down the origin of the problem. Last cycle some jobs have been red for months, and we released 22.11.00 with D10, D11, D12 marked unstable...
What will I do next cycle? All of that, and more. I will track down jenkins failures and responsibilize developers telling them when they break tests (and won't fix them anymore as I have been doing for years). I will raise on the bug reports what could have been improved. Yes, read that I will be even more annoying (to put it politely) than before.
I've noticed that the pre-commit git hook on the wiki has been broken for more than 3 years. And also caught some core developers that do not have it in place. I am relying on it to keep Vue files tidy so it's important to have it set up properly. I am planning to force its usage for ktd users [2]. Adding more checks to it will help us to catch inconsistencies from the beginning.
To summarize, writing code is cheap, maintaining code is way more expensive! It is easier to get the attention of developers before the patches are pushed to master than after, so we could be more ambitious and ask more.
For discussion :)
Cheers, Jonathan
[1] If you are using vim, open ~/vimrc, add vmap <F8> :!perltidy -q<CR> Reload vim, select code in visual mode [2] https://gitlab.com/koha-community/koha-misc4dev/-/issues/59 _______________________________________________ Koha-devel mailing list Koha-devel@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-devel website : https://www.koha-community.org/ git : https://git.koha-community.org/ bugs : https://bugs.koha-community.org/
-- Julian Maurice BibLibre _______________________________________________ Koha-devel mailing list Koha-devel@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-devel website : https://www.koha-community.org/ git : https://git.koha-community.org/ bugs : https://bugs.koha-community.org/
From what Julian write, I don't think he's suggesting running jenkins every type a patch is attached, but more something where a developper could send a message "hey, bot, can you check if this patch passes all tests ?". that would be manual (and I have no idea how to do that ;) My 1 cent (not worth 2 ;) ) Le 05/12/2022 à 14:55, Jonathan Druart a écrit :
I don't think we should run the whole test suite everytime we attach patches, that would be very expensive in terms of resources. However it would be interesting to have a temporary 'master' branch that would become 'master' only if jenkins is happy. "master" would never be broken :D
Using koha-testing-docker it's very easy to run tests locally. It will be even more easier with https://gitlab.com/koha-community/koha-misc4dev/-/issues/58
Le ven. 2 déc. 2022 à 18:16, Julian Maurice <julian.maurice@biblibre.com> a écrit :
Hi,
It seems to me that several issues could be solved by having the CI run sooner, so authors can have feedback as soon as possible. Something like github's CI integration with pull requests would be amazing to detect common mistakes early and stop wasting human time. We should know if tests pass before pushing to master, not after. We could check tidyness, commit message conventions, code coverage by tests, ... all before another person have to look at it.
Also tests are not easy to run locally. They might pass on Jenkins, but they do not on my local setup, so they are basically useless to tell me if I broke something. It also makes writing new tests more difficult than it should.
If I wanted to improve Koha developer experience, I would start with that.
My 2 cents.
Le 02/12/2022 à 15:42, Jonathan Druart a écrit :
Hi devs,
I was wondering... How good is your "good enough"? It's a question I often ask myself, when writing patches or QAing yours: is it good enough to be into Koha? It does not have to be perfect or it may never reach master, but it must meet certain standards.
When I was RM I tried to put that limit quite high. Not necessarily by asking the author for improving the follow-up patches, but also by adding the missing bits myself.
There are various types of "good enough", depending on what we look at: good enough to be reviewed, good enough to be tested, to be put in production, get feedback, try an idea, etc.
Most of the time, what is driving the limit is answering the questions "Is it maintainable?" / "Is it future proof?". Making sure the code you are writing won't be broken inadvertently is very important and must be part of the reflection.
Katrin asked the QA team members what were our plans for 23.05. In my opinion we should enforce this "be future proof" concept. Writing code is easy, especially in Koha (yes it is!). Writing maintainable and robust code, following our guidelines is a bit harder. That's why we have a QA process that tries to catch inconsistencies or edge cases you may have missed. But I think we can be even more rigorous, and aim for better standards. We can elevate our ambitions and do better. When we see the number of new enhancements we are releasing every 6 months, it shows well that writing code is definitely not a problem. However sometimes developers are tempted to abandon their work once they are pushed to master. Pushed does not mean "done". Providing bug fixes, following-up with patches when needed, fixing CI jenkins, etc. is part of job (/fun)
As a Koha developer for a long time now, I know how frustrating it can be to be asked for follow-ups/rewrite/tests to have our patches stamped with the precious PQA mark. But from the other point of view (RM, RMaints, QA team), I also know it's very frustrating when you are in charge of the release and you do not get the appropriate follow-up work once it's pushed to master.
There are some easy steps to write/review better patches. All have been discussed already several times, but that can be enforced even more: 1. Perltidy (!) This is really a very trivial step. Please perltidy your code. There are hundreds of commits that have been pushed in the last months that are not tidy (alignment, indentation, lines too long, etc.) This can easily be configured in your IDE! [1]
2. Provide clean code. As said it's not necessarily easy, but the QA team and RM are supposed to know if the code is clean regarding Koha guidelines. If the code is not clean, don't PQA, don't push. Either clean yourself, or ask the original author of the patch to do it (explaining to them how it can be improved ofc).
3. Squash! I have been away for a couple of months and had to read the git history to know what I missed. And it was really hard to follow what was going on. First of all, we are not consistent: the commit message must tell what the patch is doing, not what the bug was (if you are writing a bug fix). Then, there are way too many follow-ups: tidiness, indentation fix, typo, spelling, etc. All those tiny follow-ups could be squashed into the original patch. We don't need unnecessary tons of entries in our git log for that. For instance, I usually add a "JD Amended patch: perltidy" for instance when I tidy the original patch, to keep track of the modification. Squash can be done by the original author, the QAer, the RM. So yes, you are losing one commit in the stats but the git log is easy to read! We could have an "Amended-by" marker if we really want to add credit on the dashboard (and/or release notes).
4. Run tests. Don't wait for Jenkins to fail. This is valid for the author and QA. Anticipate the failures by running more tests. If you are modifying C4::Circulation, then run prove on t/db_dependent/Circulation*, not only Circulation.t. It will help you catch edge cases. When something is pushed, track down jenkins failures that could be caused by your patches.
6. Be strict if you are QAing. Each QA member has their own "good enough", and the RM as well (either relying on the QAer or providing a full review). But QA must fail if the code is old Koha style code, or not "good enough".
7. Provide support for failing tests, fix things you broke. The QA team will be more comfortable with your patches if you show them you are providing support for your stuff. It's not because it's pushed that you don't have any more efforts to make. Provide follow-up patches you promised, provide bug fixes, etc. We don't have a good way to keep track of such demands, which does not make tracking easier for devs, QA and RM. Any suggestions?
8. QA team MUST NEVER* pass QA a change that is not covered by tests, never. You should not provide change to modules without tests! * almost never...
9. Stick to existing patterns. We should not have different ways to do the same thing. We should not have different places where a code is doing the same thing. Ask for help or advice on the list or IRC before you start coding. We will be happy to guide you. Even if you are a regular Koha developer it's not always easy to be aware of the latest master changes. We will tell you what's the current good practice, or point you to examples you could reuse for what you want to implement.
10. CI should drive the pushes. No more push if CI is not green. The more we wait the harder it is to track down the origin of the problem. Last cycle some jobs have been red for months, and we released 22.11.00 with D10, D11, D12 marked unstable...
What will I do next cycle? All of that, and more. I will track down jenkins failures and responsibilize developers telling them when they break tests (and won't fix them anymore as I have been doing for years). I will raise on the bug reports what could have been improved. Yes, read that I will be even more annoying (to put it politely) than before.
I've noticed that the pre-commit git hook on the wiki has been broken for more than 3 years. And also caught some core developers that do not have it in place. I am relying on it to keep Vue files tidy so it's important to have it set up properly. I am planning to force its usage for ktd users [2]. Adding more checks to it will help us to catch inconsistencies from the beginning.
To summarize, writing code is cheap, maintaining code is way more expensive! It is easier to get the attention of developers before the patches are pushed to master than after, so we could be more ambitious and ask more.
For discussion :)
Cheers, Jonathan
[1] If you are using vim, open ~/vimrc, add vmap <F8> :!perltidy -q<CR> Reload vim, select code in visual mode [2] https://gitlab.com/koha-community/koha-misc4dev/-/issues/59 _______________________________________________ Koha-devel mailing list Koha-devel@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-devel website : https://www.koha-community.org/ git : https://git.koha-community.org/ bugs : https://bugs.koha-community.org/ -- Julian Maurice BibLibre
Koha-devel mailing list Koha-devel@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-devel website : https://www.koha-community.org/ git : https://git.koha-community.org/ bugs : https://bugs.koha-community.org/
Koha-devel mailing list Koha-devel@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-devel website : https://www.koha-community.org/ git : https://git.koha-community.org/ bugs : https://bugs.koha-community.org/
-- Paul Poulain, Associé-gérant / co-owner BibLibre, Services en logiciels libres pour les bibliothèques BibLibre, Open Source software and services for libraries
Yes it is expensive. But manual testing is much more expensive. I'd rather pay for some CPU time than do manually what a bot can do better and faster than me. Free services exist too (CircleCI, github actions, ...) Pushing to a temporary master branch is not a bad idea (a never-broken master branch sounds nice), but I think it would happen too late. Patches would have already been rebased multiple times, tested and reviewed before we notice a test failure. koha-testing-docker feels more like a symptom of the difficulty to run tests than a satisfying solution. It is probably necessary in order to run complicated end-to-end tests, but it should not be mandatory to run simple unit tests. And ktd is not that easy to use. It can break, and it's not easy to debug for someone not familiar with docker. Don't you think it would be a lot easier if we could run `prove t` (or `npm test` or whatever) on any dev setup and have the same failures/successes as Jenkins (minus the complicated tests like those that require selenium) ? Of course making that happen is not an easy task, but it should be a long term goal IMO. Also I think no one wants to be the person that refuse a patch because a comment is misaligned, so if that kind of thing is not automated, "not good enough code" will continue to be pushed. Le 05/12/2022 à 14:55, Jonathan Druart a écrit :
I don't think we should run the whole test suite everytime we attach patches, that would be very expensive in terms of resources. However it would be interesting to have a temporary 'master' branch that would become 'master' only if jenkins is happy. "master" would never be broken :D
Using koha-testing-docker it's very easy to run tests locally. It will be even more easier with https://gitlab.com/koha-community/koha-misc4dev/-/issues/58
Le ven. 2 déc. 2022 à 18:16, Julian Maurice <julian.maurice@biblibre.com> a écrit :
Hi,
It seems to me that several issues could be solved by having the CI run sooner, so authors can have feedback as soon as possible. Something like github's CI integration with pull requests would be amazing to detect common mistakes early and stop wasting human time. We should know if tests pass before pushing to master, not after. We could check tidyness, commit message conventions, code coverage by tests, ... all before another person have to look at it.
Also tests are not easy to run locally. They might pass on Jenkins, but they do not on my local setup, so they are basically useless to tell me if I broke something. It also makes writing new tests more difficult than it should.
If I wanted to improve Koha developer experience, I would start with that.
My 2 cents.
Le 02/12/2022 à 15:42, Jonathan Druart a écrit :
Hi devs,
I was wondering... How good is your "good enough"? It's a question I often ask myself, when writing patches or QAing yours: is it good enough to be into Koha? It does not have to be perfect or it may never reach master, but it must meet certain standards.
When I was RM I tried to put that limit quite high. Not necessarily by asking the author for improving the follow-up patches, but also by adding the missing bits myself.
There are various types of "good enough", depending on what we look at: good enough to be reviewed, good enough to be tested, to be put in production, get feedback, try an idea, etc.
Most of the time, what is driving the limit is answering the questions "Is it maintainable?" / "Is it future proof?". Making sure the code you are writing won't be broken inadvertently is very important and must be part of the reflection.
Katrin asked the QA team members what were our plans for 23.05. In my opinion we should enforce this "be future proof" concept. Writing code is easy, especially in Koha (yes it is!). Writing maintainable and robust code, following our guidelines is a bit harder. That's why we have a QA process that tries to catch inconsistencies or edge cases you may have missed. But I think we can be even more rigorous, and aim for better standards. We can elevate our ambitions and do better. When we see the number of new enhancements we are releasing every 6 months, it shows well that writing code is definitely not a problem. However sometimes developers are tempted to abandon their work once they are pushed to master. Pushed does not mean "done". Providing bug fixes, following-up with patches when needed, fixing CI jenkins, etc. is part of job (/fun)
As a Koha developer for a long time now, I know how frustrating it can be to be asked for follow-ups/rewrite/tests to have our patches stamped with the precious PQA mark. But from the other point of view (RM, RMaints, QA team), I also know it's very frustrating when you are in charge of the release and you do not get the appropriate follow-up work once it's pushed to master.
There are some easy steps to write/review better patches. All have been discussed already several times, but that can be enforced even more: 1. Perltidy (!) This is really a very trivial step. Please perltidy your code. There are hundreds of commits that have been pushed in the last months that are not tidy (alignment, indentation, lines too long, etc.) This can easily be configured in your IDE! [1]
2. Provide clean code. As said it's not necessarily easy, but the QA team and RM are supposed to know if the code is clean regarding Koha guidelines. If the code is not clean, don't PQA, don't push. Either clean yourself, or ask the original author of the patch to do it (explaining to them how it can be improved ofc).
3. Squash! I have been away for a couple of months and had to read the git history to know what I missed. And it was really hard to follow what was going on. First of all, we are not consistent: the commit message must tell what the patch is doing, not what the bug was (if you are writing a bug fix). Then, there are way too many follow-ups: tidiness, indentation fix, typo, spelling, etc. All those tiny follow-ups could be squashed into the original patch. We don't need unnecessary tons of entries in our git log for that. For instance, I usually add a "JD Amended patch: perltidy" for instance when I tidy the original patch, to keep track of the modification. Squash can be done by the original author, the QAer, the RM. So yes, you are losing one commit in the stats but the git log is easy to read! We could have an "Amended-by" marker if we really want to add credit on the dashboard (and/or release notes).
4. Run tests. Don't wait for Jenkins to fail. This is valid for the author and QA. Anticipate the failures by running more tests. If you are modifying C4::Circulation, then run prove on t/db_dependent/Circulation*, not only Circulation.t. It will help you catch edge cases. When something is pushed, track down jenkins failures that could be caused by your patches.
6. Be strict if you are QAing. Each QA member has their own "good enough", and the RM as well (either relying on the QAer or providing a full review). But QA must fail if the code is old Koha style code, or not "good enough".
7. Provide support for failing tests, fix things you broke. The QA team will be more comfortable with your patches if you show them you are providing support for your stuff. It's not because it's pushed that you don't have any more efforts to make. Provide follow-up patches you promised, provide bug fixes, etc. We don't have a good way to keep track of such demands, which does not make tracking easier for devs, QA and RM. Any suggestions?
8. QA team MUST NEVER* pass QA a change that is not covered by tests, never. You should not provide change to modules without tests! * almost never...
9. Stick to existing patterns. We should not have different ways to do the same thing. We should not have different places where a code is doing the same thing. Ask for help or advice on the list or IRC before you start coding. We will be happy to guide you. Even if you are a regular Koha developer it's not always easy to be aware of the latest master changes. We will tell you what's the current good practice, or point you to examples you could reuse for what you want to implement.
10. CI should drive the pushes. No more push if CI is not green. The more we wait the harder it is to track down the origin of the problem. Last cycle some jobs have been red for months, and we released 22.11.00 with D10, D11, D12 marked unstable...
What will I do next cycle? All of that, and more. I will track down jenkins failures and responsibilize developers telling them when they break tests (and won't fix them anymore as I have been doing for years). I will raise on the bug reports what could have been improved. Yes, read that I will be even more annoying (to put it politely) than before.
I've noticed that the pre-commit git hook on the wiki has been broken for more than 3 years. And also caught some core developers that do not have it in place. I am relying on it to keep Vue files tidy so it's important to have it set up properly. I am planning to force its usage for ktd users [2]. Adding more checks to it will help us to catch inconsistencies from the beginning.
To summarize, writing code is cheap, maintaining code is way more expensive! It is easier to get the attention of developers before the patches are pushed to master than after, so we could be more ambitious and ask more.
For discussion :)
Cheers, Jonathan
[1] If you are using vim, open ~/vimrc, add vmap <F8> :!perltidy -q<CR> Reload vim, select code in visual mode [2] https://gitlab.com/koha-community/koha-misc4dev/-/issues/59 _______________________________________________ Koha-devel mailing list Koha-devel@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-devel website : https://www.koha-community.org/ git : https://git.koha-community.org/ bugs : https://bugs.koha-community.org/
-- Julian Maurice BibLibre _______________________________________________ Koha-devel mailing list Koha-devel@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-devel website : https://www.koha-community.org/ git : https://git.koha-community.org/ bugs : https://bugs.koha-community.org/
_______________________________________________ Koha-devel mailing list Koha-devel@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-devel website : https://www.koha-community.org/ git : https://git.koha-community.org/ bugs : https://bugs.koha-community.org/
-- Julian Maurice BibLibre
Having the hooks installed automatically will help devs catch small inconsistencies, that's a first step for the easy-to-catch things. The run_tests.pl script in misc4dev will hopefully help to run the tests by anybody. Running the tests on a DB without the sample data or with existing data is already a goal, it's something we are trying to catch during QA. TestBuilder helped us a lot for that. Create the data the tests need and the cleanup/rollback should be the way to write tests. Le lun. 5 déc. 2022 à 18:24, Julian Maurice <julian.maurice@biblibre.com> a écrit :
Yes it is expensive. But manual testing is much more expensive. I'd rather pay for some CPU time than do manually what a bot can do better and faster than me. Free services exist too (CircleCI, github actions, ...)
Pushing to a temporary master branch is not a bad idea (a never-broken master branch sounds nice), but I think it would happen too late. Patches would have already been rebased multiple times, tested and reviewed before we notice a test failure.
koha-testing-docker feels more like a symptom of the difficulty to run tests than a satisfying solution. It is probably necessary in order to run complicated end-to-end tests, but it should not be mandatory to run simple unit tests. And ktd is not that easy to use. It can break, and it's not easy to debug for someone not familiar with docker.
Don't you think it would be a lot easier if we could run `prove t` (or `npm test` or whatever) on any dev setup and have the same failures/successes as Jenkins (minus the complicated tests like those that require selenium) ? Of course making that happen is not an easy task, but it should be a long term goal IMO.
Also I think no one wants to be the person that refuse a patch because a comment is misaligned, so if that kind of thing is not automated, "not good enough code" will continue to be pushed.
I don't think we should run the whole test suite everytime we attach patches, that would be very expensive in terms of resources. However it would be interesting to have a temporary 'master' branch that would become 'master' only if jenkins is happy. "master" would never be broken :D
Using koha-testing-docker it's very easy to run tests locally. It will be even more easier with https://gitlab.com/koha-community/koha-misc4dev/-/issues/58
Le ven. 2 déc. 2022 à 18:16, Julian Maurice <julian.maurice@biblibre.com> a écrit :
Hi,
It seems to me that several issues could be solved by having the CI run sooner, so authors can have feedback as soon as possible. Something like github's CI integration with pull requests would be amazing to detect common mistakes early and stop wasting human time. We should know if tests pass before pushing to master, not after. We could check tidyness, commit message conventions, code coverage by tests, ... all before another person have to look at it.
Also tests are not easy to run locally. They might pass on Jenkins, but they do not on my local setup, so they are basically useless to tell me if I broke something. It also makes writing new tests more difficult than it should.
If I wanted to improve Koha developer experience, I would start with
Le 05/12/2022 à 14:55, Jonathan Druart a écrit : that.
My 2 cents.
Le 02/12/2022 à 15:42, Jonathan Druart a écrit :
Hi devs,
I was wondering... How good is your "good enough"? It's a question I often ask myself, when writing patches or QAing yours: is it good enough to be into Koha? It does not have to be perfect or it may never reach master, but it must meet certain standards.
When I was RM I tried to put that limit quite high. Not necessarily by asking the author for improving the follow-up patches, but also by adding the missing bits myself.
There are various types of "good enough", depending on what we look at: good enough to be reviewed, good enough to be tested, to be put in production, get feedback, try an idea, etc.
Most of the time, what is driving the limit is answering the questions "Is it maintainable?" / "Is it future proof?". Making sure the code you are writing won't be broken inadvertently is very important and must be part of the reflection.
Katrin asked the QA team members what were our plans for 23.05. In my opinion we should enforce this "be future proof" concept. Writing code is easy, especially in Koha (yes it is!). Writing maintainable and robust code, following our guidelines is a bit harder. That's why we have a QA process that tries to catch inconsistencies or edge cases you may have missed. But I think we can be even more rigorous, and aim for better standards. We can elevate our ambitions and do better. When we see the number of new enhancements we are releasing every 6 months, it shows well that writing code is definitely not a problem. However sometimes developers are tempted to abandon their work once they are pushed to master. Pushed does not mean "done". Providing bug fixes, following-up with patches when needed, fixing CI jenkins, etc. is part of job (/fun)
As a Koha developer for a long time now, I know how frustrating it can be to be asked for follow-ups/rewrite/tests to have our patches stamped with the precious PQA mark. But from the other point of view (RM, RMaints, QA team), I also know it's very frustrating when you are in charge of the release and you do not get the appropriate follow-up work once it's pushed to master.
There are some easy steps to write/review better patches. All have been discussed already several times, but that can be enforced even more: 1. Perltidy (!) This is really a very trivial step. Please perltidy your code. There are hundreds of commits that have been pushed in the last months that are not tidy (alignment, indentation, lines too long, etc.) This can easily be configured in your IDE! [1]
2. Provide clean code. As said it's not necessarily easy, but the QA team and RM are supposed to know if the code is clean regarding Koha guidelines. If the code is not clean, don't PQA, don't push. Either clean yourself, or ask the original author of the patch to do it (explaining to them how it can be improved ofc).
3. Squash! I have been away for a couple of months and had to read the git history to know what I missed. And it was really hard to follow what was going on. First of all, we are not consistent: the commit message must tell what the patch is doing, not what the bug was (if you are writing a bug fix). Then, there are way too many follow-ups: tidiness, indentation fix, typo, spelling, etc. All those tiny follow-ups could be squashed into the original patch. We don't need unnecessary tons of entries in our git log for that. For instance, I usually add a "JD Amended patch: perltidy" for instance when I tidy the original patch, to keep track of the modification. Squash can be done by the original author, the QAer, the RM. So yes, you are losing one commit in the stats but the git log is easy to read! We could have an "Amended-by" marker if we really want to add credit on the dashboard (and/or release notes).
4. Run tests. Don't wait for Jenkins to fail. This is valid for the author and QA. Anticipate the failures by running more tests. If you are modifying C4::Circulation, then run prove on t/db_dependent/Circulation*, not only Circulation.t. It will help you catch edge cases. When something is pushed, track down jenkins failures that could be caused by your patches.
6. Be strict if you are QAing. Each QA member has their own "good enough", and the RM as well (either relying on the QAer or providing a full review). But QA must fail if the code is old Koha style code, or not "good enough".
7. Provide support for failing tests, fix things you broke. The QA team will be more comfortable with your patches if you show them you are providing support for your stuff. It's not because it's pushed that you don't have any more efforts to make. Provide follow-up patches you promised, provide bug fixes, etc. We don't have a good way to keep track of such demands, which does not make tracking easier for devs, QA and RM. Any suggestions?
8. QA team MUST NEVER* pass QA a change that is not covered by tests, never. You should not provide change to modules without tests! * almost never...
9. Stick to existing patterns. We should not have different ways to do the same thing. We should not have different places where a code is doing the same thing. Ask for help or advice on the list or IRC before you start coding. We will be happy to guide you. Even if you are a regular Koha developer it's not always easy to be aware of the latest master changes. We will tell you what's the current good practice, or point you to examples you could reuse for what you want to implement.
10. CI should drive the pushes. No more push if CI is not green. The more we wait the harder it is to track down the origin of the problem. Last cycle some jobs have been red for months, and we released 22.11.00 with D10, D11, D12 marked unstable...
What will I do next cycle? All of that, and more. I will track down jenkins failures and responsibilize developers telling them when they break tests (and won't fix them anymore as I have been doing for years). I will raise on the bug reports what could have been improved. Yes, read that I will be even more annoying (to put it politely) than before.
I've noticed that the pre-commit git hook on the wiki has been broken for more than 3 years. And also caught some core developers that do not have it in place. I am relying on it to keep Vue files tidy so it's important to have it set up properly. I am planning to force its usage for ktd users [2]. Adding more checks to it will help us to catch inconsistencies from the beginning.
To summarize, writing code is cheap, maintaining code is way more expensive! It is easier to get the attention of developers before the patches are pushed to master than after, so we could be more ambitious and ask more.
For discussion :)
Cheers, Jonathan
[1] If you are using vim, open ~/vimrc, add vmap <F8> :!perltidy -q<CR> Reload vim, select code in visual mode [2] https://gitlab.com/koha-community/koha-misc4dev/-/issues/59 _______________________________________________ Koha-devel mailing list Koha-devel@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-devel website : https://www.koha-community.org/ git : https://git.koha-community.org/ bugs : https://bugs.koha-community.org/
-- Julian Maurice BibLibre _______________________________________________ Koha-devel mailing list Koha-devel@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-devel website : https://www.koha-community.org/ git : https://git.koha-community.org/ bugs : https://bugs.koha-community.org/
_______________________________________________ Koha-devel mailing list Koha-devel@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-devel website : https://www.koha-community.org/ git : https://git.koha-community.org/ bugs : https://bugs.koha-community.org/
-- Julian Maurice BibLibre _______________________________________________ Koha-devel mailing list Koha-devel@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-devel website : https://www.koha-community.org/ git : https://git.koha-community.org/ bugs : https://bugs.koha-community.org/
El lun, 5 dic 2022 a las 14:24, Julian Maurice (<julian.maurice@biblibre.com>) escribió:
Yes it is expensive. But manual testing is much more expensive. I'd rather pay for some CPU time than do manually what a bot can do better and faster than me. Free services exist too (CircleCI, github actions, ...)
When you develop, you usually add some tests and run them, along with related ones that could be broken because of the changes. Running the full test suite is not mandatory. Pushing to a branch somewhere there's CI setup to run the full test suite sounds like a good idea.
Pushing to a temporary master branch is not a bad idea (a never-broken master branch sounds nice), but I think it would happen too late. Patches would have already been rebased multiple times, tested and reviewed before we notice a test failure.
Agreed!
koha-testing-docker feels more like a symptom of the difficulty to run tests than a satisfying solution. It is probably necessary in order to run complicated end-to-end tests, but it should not be mandatory to run simple unit tests. And ktd is not that easy to use. It can break, and it's not easy to debug for someone not familiar with docker.
I don't think it is mandatory, but it is the most convenient way to run in a controlled environment that matches as much as possible the conditions in production. That said, I'm pretty sure we could rely on carton or local::lib to generate an environment to work locally.
Don't you think it would be a lot easier if we could run `prove t` (or `npm test` or whatever) on any dev setup and have the same failures/successes as Jenkins (minus the complicated tests like those that require selenium) ? Of course making that happen is not an easy task, but it should be a long term goal IMO.
Also I think no one wants to be the person that refuse a patch because a comment is misaligned, so if that kind of thing is not automated, "not good enough code" will continue to be pushed.
Le 05/12/2022 à 14:55, Jonathan Druart a écrit :
I don't think we should run the whole test suite everytime we attach patches, that would be very expensive in terms of resources. However it would be interesting to have a temporary 'master' branch that would become 'master' only if jenkins is happy. "master" would never be broken :D
That would only save us some reverts if the authors fail to provide fixes. But I don't think reverts happen so often we need to change our workflow to avoid them. And jenkins to fail is the goal of it. We don't need it green all day, we need it to catch the errors (lots of errors are not covered by tests, and that's something we need to invest time on) and devs to take responsibility of fixing their breakages. I will take the blame there, because Jenkins notifications are not reaching the devs inboxes, and we need it fixed ASAP. -- Tomás Cohen Arazi Theke Solutions (http://theke.io) ✆ +54 9351 3513384 GPG: B2F3C15F
Could you provide more info on those git hooks? A wiki URL? Marcel -----Original Message----- From: Koha-devel <koha-devel-bounces@lists.koha-community.org> On Behalf Of Jonathan Druart Sent: Friday, December 2, 2022 3:43 PM To: koha-devel <koha-devel@lists.koha-community.org> Subject: [Koha-devel] Good enough? Hi devs, I was wondering... How good is your "good enough"? It's a question I often ask myself, when writing patches or QAing yours: is it good enough to be into Koha? It does not have to be perfect or it may never reach master, but it must meet certain standards. When I was RM I tried to put that limit quite high. Not necessarily by asking the author for improving the follow-up patches, but also by adding the missing bits myself. There are various types of "good enough", depending on what we look at: good enough to be reviewed, good enough to be tested, to be put in production, get feedback, try an idea, etc. Most of the time, what is driving the limit is answering the questions "Is it maintainable?" / "Is it future proof?". Making sure the code you are writing won't be broken inadvertently is very important and must be part of the reflection. Katrin asked the QA team members what were our plans for 23.05. In my opinion we should enforce this "be future proof" concept. Writing code is easy, especially in Koha (yes it is!). Writing maintainable and robust code, following our guidelines is a bit harder. That's why we have a QA process that tries to catch inconsistencies or edge cases you may have missed. But I think we can be even more rigorous, and aim for better standards. We can elevate our ambitions and do better. When we see the number of new enhancements we are releasing every 6 months, it shows well that writing code is definitely not a problem. However sometimes developers are tempted to abandon their work once they are pushed to master. Pushed does not mean "done". Providing bug fixes, following-up with patches when needed, fixing CI jenkins, etc. is part of job (/fun) As a Koha developer for a long time now, I know how frustrating it can be to be asked for follow-ups/rewrite/tests to have our patches stamped with the precious PQA mark. But from the other point of view (RM, RMaints, QA team), I also know it's very frustrating when you are in charge of the release and you do not get the appropriate follow-up work once it's pushed to master. There are some easy steps to write/review better patches. All have been discussed already several times, but that can be enforced even more: 1. Perltidy (!) This is really a very trivial step. Please perltidy your code. There are hundreds of commits that have been pushed in the last months that are not tidy (alignment, indentation, lines too long, etc.) This can easily be configured in your IDE! [1] 2. Provide clean code. As said it's not necessarily easy, but the QA team and RM are supposed to know if the code is clean regarding Koha guidelines. If the code is not clean, don't PQA, don't push. Either clean yourself, or ask the original author of the patch to do it (explaining to them how it can be improved ofc). 3. Squash! I have been away for a couple of months and had to read the git history to know what I missed. And it was really hard to follow what was going on. First of all, we are not consistent: the commit message must tell what the patch is doing, not what the bug was (if you are writing a bug fix). Then, there are way too many follow-ups: tidiness, indentation fix, typo, spelling, etc. All those tiny follow-ups could be squashed into the original patch. We don't need unnecessary tons of entries in our git log for that. For instance, I usually add a "JD Amended patch: perltidy" for instance when I tidy the original patch, to keep track of the modification. Squash can be done by the original author, the QAer, the RM. So yes, you are losing one commit in the stats but the git log is easy to read! We could have an "Amended-by" marker if we really want to add credit on the dashboard (and/or release notes). 4. Run tests. Don't wait for Jenkins to fail. This is valid for the author and QA. Anticipate the failures by running more tests. If you are modifying C4::Circulation, then run prove on t/db_dependent/Circulation*, not only Circulation.t. It will help you catch edge cases. When something is pushed, track down jenkins failures that could be caused by your patches. 6. Be strict if you are QAing. Each QA member has their own "good enough", and the RM as well (either relying on the QAer or providing a full review). But QA must fail if the code is old Koha style code, or not "good enough". 7. Provide support for failing tests, fix things you broke. The QA team will be more comfortable with your patches if you show them you are providing support for your stuff. It's not because it's pushed that you don't have any more efforts to make. Provide follow-up patches you promised, provide bug fixes, etc. We don't have a good way to keep track of such demands, which does not make tracking easier for devs, QA and RM. Any suggestions? 8. QA team MUST NEVER* pass QA a change that is not covered by tests, never. You should not provide change to modules without tests! * almost never... 9. Stick to existing patterns. We should not have different ways to do the same thing. We should not have different places where a code is doing the same thing. Ask for help or advice on the list or IRC before you start coding. We will be happy to guide you. Even if you are a regular Koha developer it's not always easy to be aware of the latest master changes. We will tell you what's the current good practice, or point you to examples you could reuse for what you want to implement. 10. CI should drive the pushes. No more push if CI is not green. The more we wait the harder it is to track down the origin of the problem. Last cycle some jobs have been red for months, and we released 22.11.00 with D10, D11, D12 marked unstable... What will I do next cycle? All of that, and more. I will track down jenkins failures and responsibilize developers telling them when they break tests (and won't fix them anymore as I have been doing for years). I will raise on the bug reports what could have been improved. Yes, read that I will be even more annoying (to put it politely) than before. I've noticed that the pre-commit git hook on the wiki has been broken for more than 3 years. And also caught some core developers that do not have it in place. I am relying on it to keep Vue files tidy so it's important to have it set up properly. I am planning to force its usage for ktd users [2]. Adding more checks to it will help us to catch inconsistencies from the beginning. To summarize, writing code is cheap, maintaining code is way more expensive! It is easier to get the attention of developers before the patches are pushed to master than after, so we could be more ambitious and ask more. For discussion :) Cheers, Jonathan [1] If you are using vim, open ~/vimrc, add vmap <F8> :!perltidy -q<CR> Reload vim, select code in visual mode [2] https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitlab.com... _______________________________________________ Koha-devel mailing list Koha-devel@lists.koha-community.org https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.koha... website : https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.koha-c... git : https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.koha-c... bugs : https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugs.koha-...
Existing hooks can be found at https://wiki.koha-community.org/wiki/Tips_and_tricks#Hooks Le lun. 5 déc. 2022 à 11:34, Marcel de Rooy <M.de.Rooy@rijksmuseum.nl> a écrit :
Could you provide more info on those git hooks? A wiki URL?
Marcel
-----Original Message----- From: Koha-devel <koha-devel-bounces@lists.koha-community.org> On Behalf Of Jonathan Druart Sent: Friday, December 2, 2022 3:43 PM To: koha-devel <koha-devel@lists.koha-community.org> Subject: [Koha-devel] Good enough?
Hi devs,
I was wondering... How good is your "good enough"? It's a question I often ask myself, when writing patches or QAing yours: is it good enough to be into Koha? It does not have to be perfect or it may never reach master, but it must meet certain standards.
When I was RM I tried to put that limit quite high. Not necessarily by asking the author for improving the follow-up patches, but also by adding the missing bits myself.
There are various types of "good enough", depending on what we look at: good enough to be reviewed, good enough to be tested, to be put in production, get feedback, try an idea, etc.
Most of the time, what is driving the limit is answering the questions "Is it maintainable?" / "Is it future proof?". Making sure the code you are writing won't be broken inadvertently is very important and must be part of the reflection.
Katrin asked the QA team members what were our plans for 23.05. In my opinion we should enforce this "be future proof" concept. Writing code is easy, especially in Koha (yes it is!). Writing maintainable and robust code, following our guidelines is a bit harder. That's why we have a QA process that tries to catch inconsistencies or edge cases you may have missed. But I think we can be even more rigorous, and aim for better standards. We can elevate our ambitions and do better. When we see the number of new enhancements we are releasing every 6 months, it shows well that writing code is definitely not a problem. However sometimes developers are tempted to abandon their work once they are pushed to master. Pushed does not mean "done". Providing bug fixes, following-up with patches when needed, fixing CI jenkins, etc. is part of job (/fun)
As a Koha developer for a long time now, I know how frustrating it can be to be asked for follow-ups/rewrite/tests to have our patches stamped with the precious PQA mark. But from the other point of view (RM, RMaints, QA team), I also know it's very frustrating when you are in charge of the release and you do not get the appropriate follow-up work once it's pushed to master.
There are some easy steps to write/review better patches. All have been discussed already several times, but that can be enforced even more: 1. Perltidy (!) This is really a very trivial step. Please perltidy your code. There are hundreds of commits that have been pushed in the last months that are not tidy (alignment, indentation, lines too long, etc.) This can easily be configured in your IDE! [1]
2. Provide clean code. As said it's not necessarily easy, but the QA team and RM are supposed to know if the code is clean regarding Koha guidelines. If the code is not clean, don't PQA, don't push. Either clean yourself, or ask the original author of the patch to do it (explaining to them how it can be improved ofc).
3. Squash! I have been away for a couple of months and had to read the git history to know what I missed. And it was really hard to follow what was going on. First of all, we are not consistent: the commit message must tell what the patch is doing, not what the bug was (if you are writing a bug fix). Then, there are way too many follow-ups: tidiness, indentation fix, typo, spelling, etc. All those tiny follow-ups could be squashed into the original patch. We don't need unnecessary tons of entries in our git log for that. For instance, I usually add a "JD Amended patch: perltidy" for instance when I tidy the original patch, to keep track of the modification. Squash can be done by the original author, the QAer, the RM. So yes, you are losing one commit in the stats but the git log is easy to read! We could have an "Amended-by" marker if we really want to add credit on the dashboard (and/or release notes).
4. Run tests. Don't wait for Jenkins to fail. This is valid for the author and QA. Anticipate the failures by running more tests. If you are modifying C4::Circulation, then run prove on t/db_dependent/Circulation*, not only Circulation.t. It will help you catch edge cases. When something is pushed, track down jenkins failures that could be caused by your patches.
6. Be strict if you are QAing. Each QA member has their own "good enough", and the RM as well (either relying on the QAer or providing a full review). But QA must fail if the code is old Koha style code, or not "good enough".
7. Provide support for failing tests, fix things you broke. The QA team will be more comfortable with your patches if you show them you are providing support for your stuff. It's not because it's pushed that you don't have any more efforts to make. Provide follow-up patches you promised, provide bug fixes, etc. We don't have a good way to keep track of such demands, which does not make tracking easier for devs, QA and RM. Any suggestions?
8. QA team MUST NEVER* pass QA a change that is not covered by tests, never. You should not provide change to modules without tests! * almost never...
9. Stick to existing patterns. We should not have different ways to do the same thing. We should not have different places where a code is doing the same thing. Ask for help or advice on the list or IRC before you start coding. We will be happy to guide you. Even if you are a regular Koha developer it's not always easy to be aware of the latest master changes. We will tell you what's the current good practice, or point you to examples you could reuse for what you want to implement.
10. CI should drive the pushes. No more push if CI is not green. The more we wait the harder it is to track down the origin of the problem. Last cycle some jobs have been red for months, and we released 22.11.00 with D10, D11, D12 marked unstable...
What will I do next cycle? All of that, and more. I will track down jenkins failures and responsibilize developers telling them when they break tests (and won't fix them anymore as I have been doing for years). I will raise on the bug reports what could have been improved. Yes, read that I will be even more annoying (to put it politely) than before.
I've noticed that the pre-commit git hook on the wiki has been broken for more than 3 years. And also caught some core developers that do not have it in place. I am relying on it to keep Vue files tidy so it's important to have it set up properly. I am planning to force its usage for ktd users [2]. Adding more checks to it will help us to catch inconsistencies from the beginning.
To summarize, writing code is cheap, maintaining code is way more expensive! It is easier to get the attention of developers before the patches are pushed to master than after, so we could be more ambitious and ask more.
For discussion :)
Cheers, Jonathan
[1] If you are using vim, open ~/vimrc, add vmap <F8> :!perltidy -q<CR> Reload vim, select code in visual mode [2] https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitlab.com... _______________________________________________ Koha-devel mailing list Koha-devel@lists.koha-community.org https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.koha... website : https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.koha-c... git : https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.koha-c... bugs : https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugs.koha-...
Thanks for sending this link. Could we merge that code with QA code we already pushed into our codebase somehow ? -----Original Message----- From: Koha-devel <koha-devel-bounces@lists.koha-community.org> On Behalf Of Jonathan Druart Sent: Monday, December 5, 2022 2:56 PM To: koha-devel <koha-devel@lists.koha-community.org> Subject: Re: [Koha-devel] Good enough? Existing hooks can be found at https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwiki.koha-... Le lun. 5 déc. 2022 à 11:34, Marcel de Rooy <M.de.Rooy@rijksmuseum.nl> a écrit :
Could you provide more info on those git hooks? A wiki URL?
Marcel
-----Original Message----- From: Koha-devel <koha-devel-bounces@lists.koha-community.org> On Behalf Of Jonathan Druart Sent: Friday, December 2, 2022 3:43 PM To: koha-devel <koha-devel@lists.koha-community.org> Subject: [Koha-devel] Good enough?
Hi devs,
I was wondering... How good is your "good enough"? It's a question I often ask myself, when writing patches or QAing yours: is it good enough to be into Koha? It does not have to be perfect or it may never reach master, but it must meet certain standards.
When I was RM I tried to put that limit quite high. Not necessarily by asking the author for improving the follow-up patches, but also by adding the missing bits myself.
There are various types of "good enough", depending on what we look at: good enough to be reviewed, good enough to be tested, to be put in production, get feedback, try an idea, etc.
Most of the time, what is driving the limit is answering the questions "Is it maintainable?" / "Is it future proof?". Making sure the code you are writing won't be broken inadvertently is very important and must be part of the reflection.
Katrin asked the QA team members what were our plans for 23.05. In my opinion we should enforce this "be future proof" concept. Writing code is easy, especially in Koha (yes it is!). Writing maintainable and robust code, following our guidelines is a bit harder. That's why we have a QA process that tries to catch inconsistencies or edge cases you may have missed. But I think we can be even more rigorous, and aim for better standards. We can elevate our ambitions and do better. When we see the number of new enhancements we are releasing every 6 months, it shows well that writing code is definitely not a problem. However sometimes developers are tempted to abandon their work once they are pushed to master. Pushed does not mean "done". Providing bug fixes, following-up with patches when needed, fixing CI jenkins, etc. is part of job (/fun)
As a Koha developer for a long time now, I know how frustrating it can be to be asked for follow-ups/rewrite/tests to have our patches stamped with the precious PQA mark. But from the other point of view (RM, RMaints, QA team), I also know it's very frustrating when you are in charge of the release and you do not get the appropriate follow-up work once it's pushed to master.
There are some easy steps to write/review better patches. All have been discussed already several times, but that can be enforced even more: 1. Perltidy (!) This is really a very trivial step. Please perltidy your code. There are hundreds of commits that have been pushed in the last months that are not tidy (alignment, indentation, lines too long, etc.) This can easily be configured in your IDE! [1]
2. Provide clean code. As said it's not necessarily easy, but the QA team and RM are supposed to know if the code is clean regarding Koha guidelines. If the code is not clean, don't PQA, don't push. Either clean yourself, or ask the original author of the patch to do it (explaining to them how it can be improved ofc).
3. Squash! I have been away for a couple of months and had to read the git history to know what I missed. And it was really hard to follow what was going on. First of all, we are not consistent: the commit message must tell what the patch is doing, not what the bug was (if you are writing a bug fix). Then, there are way too many follow-ups: tidiness, indentation fix, typo, spelling, etc. All those tiny follow-ups could be squashed into the original patch. We don't need unnecessary tons of entries in our git log for that. For instance, I usually add a "JD Amended patch: perltidy" for instance when I tidy the original patch, to keep track of the modification. Squash can be done by the original author, the QAer, the RM. So yes, you are losing one commit in the stats but the git log is easy to read! We could have an "Amended-by" marker if we really want to add credit on the dashboard (and/or release notes).
4. Run tests. Don't wait for Jenkins to fail. This is valid for the author and QA. Anticipate the failures by running more tests. If you are modifying C4::Circulation, then run prove on t/db_dependent/Circulation*, not only Circulation.t. It will help you catch edge cases. When something is pushed, track down jenkins failures that could be caused by your patches.
6. Be strict if you are QAing. Each QA member has their own "good enough", and the RM as well (either relying on the QAer or providing a full review). But QA must fail if the code is old Koha style code, or not "good enough".
7. Provide support for failing tests, fix things you broke. The QA team will be more comfortable with your patches if you show them you are providing support for your stuff. It's not because it's pushed that you don't have any more efforts to make. Provide follow-up patches you promised, provide bug fixes, etc. We don't have a good way to keep track of such demands, which does not make tracking easier for devs, QA and RM. Any suggestions?
8. QA team MUST NEVER* pass QA a change that is not covered by tests, never. You should not provide change to modules without tests! * almost never...
9. Stick to existing patterns. We should not have different ways to do the same thing. We should not have different places where a code is doing the same thing. Ask for help or advice on the list or IRC before you start coding. We will be happy to guide you. Even if you are a regular Koha developer it's not always easy to be aware of the latest master changes. We will tell you what's the current good practice, or point you to examples you could reuse for what you want to implement.
10. CI should drive the pushes. No more push if CI is not green. The more we wait the harder it is to track down the origin of the problem. Last cycle some jobs have been red for months, and we released 22.11.00 with D10, D11, D12 marked unstable...
What will I do next cycle? All of that, and more. I will track down jenkins failures and responsibilize developers telling them when they break tests (and won't fix them anymore as I have been doing for years). I will raise on the bug reports what could have been improved. Yes, read that I will be even more annoying (to put it politely) than before.
I've noticed that the pre-commit git hook on the wiki has been broken for more than 3 years. And also caught some core developers that do not have it in place. I am relying on it to keep Vue files tidy so it's important to have it set up properly. I am planning to force its usage for ktd users [2]. Adding more checks to it will help us to catch inconsistencies from the beginning.
To summarize, writing code is cheap, maintaining code is way more expensive! It is easier to get the attention of developers before the patches are pushed to master than after, so we could be more ambitious and ask more.
For discussion :)
Cheers, Jonathan
[1] If you are using vim, open ~/vimrc, add vmap <F8> :!perltidy -q<CR> Reload vim, select code in visual mode [2] https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitl ab.com%2Fkoha-community%2Fkoha-misc4dev%2F-%2Fissues%2F59&data=05% 7C01%7Cm.de.rooy%40rijksmuseum.nl%7C5d76e33c09514528c67808dad6c87f27%7 C635b05eb66c748e1a94fb4b05a1b058b%7C0%7C0%7C638058453872270550%7CUnkno wn%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiL CJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=AkLM4JATxwG4L8CcLpZ7z34oM6GCEN 49uAo3dt4Sw18%3D&reserved=0 _______________________________________________ Koha-devel mailing list Koha-devel@lists.koha-community.org https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Flist s.koha-community.org%2Fcgi-bin%2Fmailman%2Flistinfo%2Fkoha-devel&d ata=05%7C01%7Cm.de.rooy%40rijksmuseum.nl%7C5d76e33c09514528c67808dad6c 87f27%7C635b05eb66c748e1a94fb4b05a1b058b%7C0%7C0%7C638058453872270550% 7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik 1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=aqN6n2Z2kvsodCb1MlLsNiA AomgcvKr6fB7tCNNdBrg%3D&reserved=0 website : https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww. koha-community.org%2F&data=05%7C01%7Cm.de.rooy%40rijksmuseum.nl%7C 5d76e33c09514528c67808dad6c87f27%7C635b05eb66c748e1a94fb4b05a1b058b%7C 0%7C0%7C638058453872270550%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDA iLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sd ata=3vEx8LIuZAkid78L%2FF15H58ECkQQu4m4PO7R4Tnbot0%3D&reserved=0 git : https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit. koha-community.org%2F&data=05%7C01%7Cm.de.rooy%40rijksmuseum.nl%7C 5d76e33c09514528c67808dad6c87f27%7C635b05eb66c748e1a94fb4b05a1b058b%7C 0%7C0%7C638058453872270550%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDA iLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sd ata=5bMenLRZpyj4HYRx%2FExY83Mwe%2F8bpaLvlJvG3jLcBCg%3D&reserved=0 bugs : https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugs .koha-community.org%2F&data=05%7C01%7Cm.de.rooy%40rijksmuseum.nl%7 C5d76e33c09514528c67808dad6c87f27%7C635b05eb66c748e1a94fb4b05a1b058b%7 C0%7C0%7C638058453872270550%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMD AiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&s data=iT5Hl4dNO8NQXuQZmJmkdLr1pXv0k3TBGK8NceA82iE%3D&reserved=0
_______________________________________________ Koha-devel mailing list Koha-devel@lists.koha-community.org https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.koha... website : https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.koha-c... git : https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.koha-c... bugs : https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugs.koha-...
As said in the email I am planning to integrate them into our dev env (via misc4dev), see https://gitlab.com/koha-community/koha-misc4dev/-/issues/59 Le lun. 5 déc. 2022 à 15:17, Marcel de Rooy <M.de.Rooy@rijksmuseum.nl> a écrit :
Thanks for sending this link. Could we merge that code with QA code we already pushed into our codebase somehow ?
-----Original Message----- From: Koha-devel <koha-devel-bounces@lists.koha-community.org> On Behalf Of Jonathan Druart Sent: Monday, December 5, 2022 2:56 PM To: koha-devel <koha-devel@lists.koha-community.org> Subject: Re: [Koha-devel] Good enough?
Existing hooks can be found at https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwiki.koha-...
Le lun. 5 déc. 2022 à 11:34, Marcel de Rooy <M.de.Rooy@rijksmuseum.nl> a écrit :
Could you provide more info on those git hooks? A wiki URL?
Marcel
-----Original Message----- From: Koha-devel <koha-devel-bounces@lists.koha-community.org> On Behalf Of Jonathan Druart Sent: Friday, December 2, 2022 3:43 PM To: koha-devel <koha-devel@lists.koha-community.org> Subject: [Koha-devel] Good enough?
Hi devs,
I was wondering... How good is your "good enough"? It's a question I often ask myself, when writing patches or QAing yours: is it good enough to be into Koha? It does not have to be perfect or it may never reach master, but it must meet certain standards.
When I was RM I tried to put that limit quite high. Not necessarily by asking the author for improving the follow-up patches, but also by adding the missing bits myself.
There are various types of "good enough", depending on what we look at: good enough to be reviewed, good enough to be tested, to be put in production, get feedback, try an idea, etc.
Most of the time, what is driving the limit is answering the questions "Is it maintainable?" / "Is it future proof?". Making sure the code you are writing won't be broken inadvertently is very important and must be part of the reflection.
Katrin asked the QA team members what were our plans for 23.05. In my opinion we should enforce this "be future proof" concept. Writing code is easy, especially in Koha (yes it is!). Writing maintainable and robust code, following our guidelines is a bit harder. That's why we have a QA process that tries to catch inconsistencies or edge cases you may have missed. But I think we can be even more rigorous, and aim for better standards. We can elevate our ambitions and do better. When we see the number of new enhancements we are releasing every 6 months, it shows well that writing code is definitely not a problem. However sometimes developers are tempted to abandon their work once they are pushed to master. Pushed does not mean "done". Providing bug fixes, following-up with patches when needed, fixing CI jenkins, etc. is part of job (/fun)
As a Koha developer for a long time now, I know how frustrating it can be to be asked for follow-ups/rewrite/tests to have our patches stamped with the precious PQA mark. But from the other point of view (RM, RMaints, QA team), I also know it's very frustrating when you are in charge of the release and you do not get the appropriate follow-up work once it's pushed to master.
There are some easy steps to write/review better patches. All have been discussed already several times, but that can be enforced even more: 1. Perltidy (!) This is really a very trivial step. Please perltidy your code. There are hundreds of commits that have been pushed in the last months that are not tidy (alignment, indentation, lines too long, etc.) This can easily be configured in your IDE! [1]
2. Provide clean code. As said it's not necessarily easy, but the QA team and RM are supposed to know if the code is clean regarding Koha guidelines. If the code is not clean, don't PQA, don't push. Either clean yourself, or ask the original author of the patch to do it (explaining to them how it can be improved ofc).
3. Squash! I have been away for a couple of months and had to read the git history to know what I missed. And it was really hard to follow what was going on. First of all, we are not consistent: the commit message must tell what the patch is doing, not what the bug was (if you are writing a bug fix). Then, there are way too many follow-ups: tidiness, indentation fix, typo, spelling, etc. All those tiny follow-ups could be squashed into the original patch. We don't need unnecessary tons of entries in our git log for that. For instance, I usually add a "JD Amended patch: perltidy" for instance when I tidy the original patch, to keep track of the modification. Squash can be done by the original author, the QAer, the RM. So yes, you are losing one commit in the stats but the git log is easy to read! We could have an "Amended-by" marker if we really want to add credit on the dashboard (and/or release notes).
4. Run tests. Don't wait for Jenkins to fail. This is valid for the author and QA. Anticipate the failures by running more tests. If you are modifying C4::Circulation, then run prove on t/db_dependent/Circulation*, not only Circulation.t. It will help you catch edge cases. When something is pushed, track down jenkins failures that could be caused by your patches.
6. Be strict if you are QAing. Each QA member has their own "good enough", and the RM as well (either relying on the QAer or providing a full review). But QA must fail if the code is old Koha style code, or not "good enough".
7. Provide support for failing tests, fix things you broke. The QA team will be more comfortable with your patches if you show them you are providing support for your stuff. It's not because it's pushed that you don't have any more efforts to make. Provide follow-up patches you promised, provide bug fixes, etc. We don't have a good way to keep track of such demands, which does not make tracking easier for devs, QA and RM. Any suggestions?
8. QA team MUST NEVER* pass QA a change that is not covered by tests, never. You should not provide change to modules without tests! * almost never...
9. Stick to existing patterns. We should not have different ways to do the same thing. We should not have different places where a code is doing the same thing. Ask for help or advice on the list or IRC before you start coding. We will be happy to guide you. Even if you are a regular Koha developer it's not always easy to be aware of the latest master changes. We will tell you what's the current good practice, or point you to examples you could reuse for what you want to implement.
10. CI should drive the pushes. No more push if CI is not green. The more we wait the harder it is to track down the origin of the problem. Last cycle some jobs have been red for months, and we released 22.11.00 with D10, D11, D12 marked unstable...
What will I do next cycle? All of that, and more. I will track down jenkins failures and responsibilize developers telling them when they break tests (and won't fix them anymore as I have been doing for years). I will raise on the bug reports what could have been improved. Yes, read that I will be even more annoying (to put it politely) than before.
I've noticed that the pre-commit git hook on the wiki has been broken for more than 3 years. And also caught some core developers that do not have it in place. I am relying on it to keep Vue files tidy so it's important to have it set up properly. I am planning to force its usage for ktd users [2]. Adding more checks to it will help us to catch inconsistencies from the beginning.
To summarize, writing code is cheap, maintaining code is way more expensive! It is easier to get the attention of developers before the patches are pushed to master than after, so we could be more ambitious and ask more.
For discussion :)
Cheers, Jonathan
[1] If you are using vim, open ~/vimrc, add vmap <F8> :!perltidy -q<CR> Reload vim, select code in visual mode [2] https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitl ab.com%2Fkoha-community%2Fkoha-misc4dev%2F-%2Fissues%2F59&data=05% 7C01%7Cm.de.rooy%40rijksmuseum.nl%7C5d76e33c09514528c67808dad6c87f27%7 C635b05eb66c748e1a94fb4b05a1b058b%7C0%7C0%7C638058453872270550%7CUnkno wn%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiL CJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=AkLM4JATxwG4L8CcLpZ7z34oM6GCEN 49uAo3dt4Sw18%3D&reserved=0 _______________________________________________ Koha-devel mailing list Koha-devel@lists.koha-community.org https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Flist s.koha-community.org%2Fcgi-bin%2Fmailman%2Flistinfo%2Fkoha-devel&d ata=05%7C01%7Cm.de.rooy%40rijksmuseum.nl%7C5d76e33c09514528c67808dad6c 87f27%7C635b05eb66c748e1a94fb4b05a1b058b%7C0%7C0%7C638058453872270550% 7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik 1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=aqN6n2Z2kvsodCb1MlLsNiA AomgcvKr6fB7tCNNdBrg%3D&reserved=0 website : https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww. koha-community.org%2F&data=05%7C01%7Cm.de.rooy%40rijksmuseum.nl%7C 5d76e33c09514528c67808dad6c87f27%7C635b05eb66c748e1a94fb4b05a1b058b%7C 0%7C0%7C638058453872270550%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDA iLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sd ata=3vEx8LIuZAkid78L%2FF15H58ECkQQu4m4PO7R4Tnbot0%3D&reserved=0 git : https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit. koha-community.org%2F&data=05%7C01%7Cm.de.rooy%40rijksmuseum.nl%7C 5d76e33c09514528c67808dad6c87f27%7C635b05eb66c748e1a94fb4b05a1b058b%7C 0%7C0%7C638058453872270550%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDA iLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sd ata=5bMenLRZpyj4HYRx%2FExY83Mwe%2F8bpaLvlJvG3jLcBCg%3D&reserved=0 bugs : https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugs .koha-community.org%2F&data=05%7C01%7Cm.de.rooy%40rijksmuseum.nl%7 C5d76e33c09514528c67808dad6c87f27%7C635b05eb66c748e1a94fb4b05a1b058b%7 C0%7C0%7C638058453872270550%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMD AiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&s data=iT5Hl4dNO8NQXuQZmJmkdLr1pXv0k3TBGK8NceA82iE%3D&reserved=0
_______________________________________________ Koha-devel mailing list Koha-devel@lists.koha-community.org https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.koha... website : https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.koha-c... git : https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.koha-c... bugs : https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugs.koha-... _______________________________________________ Koha-devel mailing list Koha-devel@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-devel website : https://www.koha-community.org/ git : https://git.koha-community.org/ bugs : https://bugs.koha-community.org/
What is your dev environment like? a dev install? on the host OS or a VM? We tried hard to make the misc4dev tools agnostic of KTD. Feedback is welcome. El mar, 6 dic 2022 a las 9:17, Marcel de Rooy (<M.de.Rooy@rijksmuseum.nl>) escribió:
As said in the email I am planning to integrate them into our dev env (via misc4dev), see
Great but please note that 'our dev env' is not per definition KTD for everybody. What we do share is the git repo 🙂
Marcel _______________________________________________ Koha-devel mailing list Koha-devel@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-devel website : https://www.koha-community.org/ git : https://git.koha-community.org/ bugs : https://bugs.koha-community.org/
-- Tomás Cohen Arazi Theke Solutions (http://theke.io) ✆ +54 9351 3513384 GPG: B2F3C15F
El vie, 2 dic 2022 a las 11:43, Jonathan Druart (< jonathan.druart@bugs.koha-community.org>) escribió:
10. CI should drive the pushes. No more push if CI is not green. The more we wait the harder it is to track down the origin of the problem. Last cycle some jobs have been red for months, and we released 22.11.00 with D10, D11, D12 marked unstable...
That assertion is not that precise. We've had lots of failures during the cycle which were addressed sooner or later. The only long standing one was a selenium test failure we couldn't reproduce locally, and was more about the test suite setup than a Koh failure. And we only released once things were green :-D I will answer about the rest of the email later. But I will mention that everyone can: $ ktd --selenium --es7 --sso up -d and be able to test everything locally [1]. And I'm available to chat or troubleshoot that. [1] With the exception of macOS + ARM users. -- Tomás Cohen Arazi Theke Solutions (http://theke.io) ✆ +54 9351 3513384 GPG: B2F3C15F
participants (5)
-
Jonathan Druart -
Julian Maurice -
Marcel de Rooy -
Paul Poulain -
Tomas Cohen Arazi