Hey list,

over the past year or so, I've started to contribute to Alpine Linux's 
aports, and have met various codestyles. To get to the bottom of what is 
'the' Alpine Linux codestyle, I came up empty handed, there is no 
CODESTYLE.md, no search query that finds anything nor on the wiki.

Now of course, the document could be hiding out of plain site, and if 
that is the case, other then saying 'make it findable', I appologize for 
even bringing anything up here.

As I was about to do a version bump to community/mtd-utils, I noticed a 
lot of activity on the package, a great thing!. Sadly I came to the 
realization, that in the 'default' aports workflow, the 
maintainers/contributers do not get CC-ed on changes. As I'm mostly only 
familiar with Linux, U-Boot and busybox development, I missed the 
'get_maintainers' step, where people involved in abuild's would be kept 
in the loop.

Never the less, what triggered for me to address the mailing list 
however was commit 57d39405ad74 ("community/mtd-utils: adhere to aports 
code style"). As the subject already states, 'adhere to aports code 
style' is mentioned, but as mentioned earlier, I cannot seem to find 
this aforementioned code style. It should be obvious then of course, 
that when asking people to adhere to a coding style, this style needed 
to be documented (and more importantantly rationalized). It shouldn't 
needed to be mentioned, by having a guide (that can be checked with 
linters/checkers) makes life easier, for both developers and reviewers 
(passes automated code checks, no need to argue about style anymore).

So in short, where is the style guide that everybody keeps talking about :)

Thank you,

Olliver

On Tue Jul 23, 2019 at 8:28 AM Olliver Schinagl wrote:
> over the past year or so, I've started to contribute to Alpine Linux's 
> aports, and have met various codestyles. To get to the bottom of what is 
> 'the' Alpine Linux codestyle, I came up empty handed, there is no 
> CODESTYLE.md, no search query that finds anything nor on the wiki.

Informally I gather that the basics are:

- snake_case
- Indent with a single tab
- 1 blank line after the initial set of variables, then again between
  each function
- Wrap to 80 columns
- Local variables (e.g. not pkgname, options, etc) named with an
  underscore prefix

I also generally alpha-sort when I have lots of things which can be
arbitrarily ordered. When in doubt pick a couple of nice looking
APKBUILDs to keep as a reference. I use this page as a reference for
Python packages:

https://wiki.alpinelinux.org/wiki/Python_package_policies

abuild will also warn you on several common errors automatically. I
think there's a second package you can install somewhere which expands
the list of lintable problems, which probably ought to be merged into
abuild proper.

> As I was about to do a version bump to community/mtd-utils, I noticed a 
> lot of activity on the package, a great thing!. Sadly I came to the 
> realization, that in the 'default' aports workflow, the 
> maintainers/contributers do not get CC-ed on changes. As I'm mostly only 
> familiar with Linux, U-Boot and busybox development, I missed the 
> 'get_maintainers' step, where people involved in abuild's would be kept 
> in the loop.

I use the email based workflow and send patches to
~alpine/aports@lists.alpinelinux.org. When I do this I often copy/paste
the Maintainer and/or Contributor lines into the email and swap them for
Cc. You can get git send-email to open your editor for reviewing/editing
the email by passing --annotate or setting git config --global
sendemail.annotate yes.

On Tue, 2019-07-23 at 08:28, Olliver Schinagl wrote:
> Hey list,
> 
> over the past year or so, I've started to contribute to Alpine Linux's
> aports, and have met various codestyles. To get to the bottom of what is
> 'the' Alpine Linux codestyle, I came up empty handed, there is no
> CODESTYLE.md, no search query that finds anything nor on the wiki.

For some time I'm 'writing' this (your) mail in my head (but didn't on
keyboard).

> Now of course, the document could be hiding out of plain site, and if that
> is the case, other then saying 'make it findable', I appologize for even
> bringing anything up here.
> 
> As I was about to do a version bump to community/mtd-utils, I noticed a lot
> of activity on the package, a great thing!. Sadly I came to the realization,
> that in the 'default' aports workflow, the maintainers/contributers do not
> get CC-ed on changes. As I'm mostly only familiar with Linux, U-Boot and
> busybox development, I missed the 'get_maintainers' step, where people
> involved in abuild's would be kept in the loop.

This part should be addressed, IMO.
When changing, upgrading, fixing package we should ask maintainer (at
least) first and maybe contributor's if maintainer doesn't respond, and
not 'blindly' push changes.

Of course, guide and some rules would help for this.

> Never the less, what triggered for me to address the mailing list however
> was commit 57d39405ad74 ("community/mtd-utils: adhere to aports code
> style"). As the subject already states, 'adhere to aports code style' is
> mentioned, but as mentioned earlier, I cannot seem to find this
> aforementioned code style. It should be obvious then of course, that when
> asking people to adhere to a coding style, this style needed to be
> documented (and more importantantly rationalized). It shouldn't needed to be
> mentioned, by having a guide (that can be checked with linters/checkers)
> makes life easier, for both developers and reviewers (passes automated code
> checks, no need to argue about style anymore).
> 
> So in short, where is the style guide that everybody keeps talking about :)
> 
> Thank you,
> 
> Olliver

On Tue, 23 Jul 2019 08:28:11 +0200
Olliver Schinagl <oliver+list@schinagl.nl> wrote:

> Hey list,
> 
> over the past year or so, I've started to contribute to Alpine Linux's 
> aports, and have met various codestyles. To get to the bottom of what is 
> 'the' Alpine Linux codestyle, I came up empty handed, there is no 
> CODESTYLE.md, no search query that finds anything nor on the wiki.
> 
> Now of course, the document could be hiding out of plain site, and if 
> that is the case, other then saying 'make it findable', I appologize for 
> even bringing anything up here.

> As I was about to do a version bump to community/mtd-utils, I noticed
> a lot of activity on the package, a great thing!. Sadly I came to the 
> realization, that in the 'default' aports workflow, the 
> maintainers/contributers do not get CC-ed on changes. As I'm mostly
> only familiar with Linux, U-Boot and busybox development, I missed
> the 'get_maintainers' step, where people involved in abuild's would
> be kept in the loop.

This can and be improved.

> Never the less, what triggered for me to address the mailing list 
> however was commit 57d39405ad74 ("community/mtd-utils: adhere to
> aports code style"). As the subject already states, 'adhere to aports
> code style' is mentioned, but as mentioned earlier, I cannot seem to
> find this aforementioned code style. It should be obvious then of
> course, that when asking people to adhere to a coding style, this
> style needed to be documented (and more importantantly rationalized).
> It shouldn't needed to be mentioned, by having a guide (that can be
> checked with linters/checkers) makes life easier, for both developers
> and reviewers (passes automated code checks, no need to argue about
> style anymore).
> 
> So in short, where is the style guide that everybody keeps talking
> about :)

Its still unwritten, sorry. I guess its based on what is common for the
majority of the APKBUILDs. It is ofcourse not realistic that everyone
read all of the thousands of APKBUILDs to get how things are normally
done, so yeah, this should be fixed.

I created an issue: https://gitlab.alpinelinux.org/alpine/aports/issues/10689

> 
> Thank you,
> 
> Olliver

Hi Drew,

On 23-07-2019 16:01, Drew DeVault wrote:
> On Tue Jul 23, 2019 at 8:28 AM Olliver Schinagl wrote:
>> over the past year or so, I've started to contribute to Alpine Linux's
>> aports, and have met various codestyles. To get to the bottom of what is
>> 'the' Alpine Linux codestyle, I came up empty handed, there is no
>> CODESTYLE.md, no search query that finds anything nor on the wiki.
> 
> Informally I gather that the basics are:
> 
> - snake_case
> - Indent with a single tab
> - 1 blank line after the initial set of variables, then again between
>    each function
> - Wrap to 80 columns
> - Local variables (e.g. not pkgname, options, etc) named with an
>    underscore prefix

But that really depends on who the maintainer is, and more importantly, 
who the person is who accepts the ticket or worse, simply overwrites to 
adhere to his style. This inconsistency makes very frustrating for 
contributers, which pick an APKBUILD file as an example work on that, 
and then get told they are doing it wrong.

However, one big downside of having flexibility of course it reduces 
exclusivity and thus contributions. However arguing about style and then 
not having a contribution merged, is of course far worse.

> 
> I also generally alpha-sort when I have lots of things which can be
> arbitrarily ordered. When in doubt pick a couple of nice looking
> APKBUILDs to keep as a reference. I use this page as a reference for
> Python packages:
> 
> https://wiki.alpinelinux.org/wiki/Python_package_policies
> 
> abuild will also warn you on several common errors automatically. I
> think there's a second package you can install somewhere which expands
> the list of lintable problems, which probably ought to be merged into
> abuild proper.
> 
>> As I was about to do a version bump to community/mtd-utils, I noticed a
>> lot of activity on the package, a great thing!. Sadly I came to the
>> realization, that in the 'default' aports workflow, the
>> maintainers/contributers do not get CC-ed on changes. As I'm mostly only
>> familiar with Linux, U-Boot and busybox development, I missed the
>> 'get_maintainers' step, where people involved in abuild's would be kept
>> in the loop.
> 
> I use the email based workflow and send patches to
> ~alpine/aports@lists.alpinelinux.org. When I do this I often copy/paste
> the Maintainer and/or Contributor lines into the email and swap them for
> Cc. You can get git send-email to open your editor for reviewing/editing
> the email by passing --annotate or setting git config --global
> sendemail.annotate yes.
> 

Hey Milan,

On 23-07-2019 17:20, Milan P. Stanić wrote:
> On Tue, 2019-07-23 at 08:28, Olliver Schinagl wrote:
>> Hey list,
>>
>> over the past year or so, I've started to contribute to Alpine Linux's
>> aports, and have met various codestyles. To get to the bottom of what is
>> 'the' Alpine Linux codestyle, I came up empty handed, there is no
>> CODESTYLE.md, no search query that finds anything nor on the wiki.
> 
> For some time I'm 'writing' this (your) mail in my head (but didn't on
> keyboard).
Thanks! I think? :)

> 
>> Now of course, the document could be hiding out of plain site, and if that
>> is the case, other then saying 'make it findable', I appologize for even
>> bringing anything up here.
>>
>> As I was about to do a version bump to community/mtd-utils, I noticed a lot
>> of activity on the package, a great thing!. Sadly I came to the realization,
>> that in the 'default' aports workflow, the maintainers/contributers do not
>> get CC-ed on changes. As I'm mostly only familiar with Linux, U-Boot and
>> busybox development, I missed the 'get_maintainers' step, where people
>> involved in abuild's would be kept in the loop.
> 
> This part should be addressed, IMO.
> When changing, upgrading, fixing package we should ask maintainer (at
> least) first and maybe contributor's if maintainer doesn't respond, and
> not 'blindly' push changes.

Yeah they should be in the CC. I think gitlab has the 'maintainers' 
feature for this, sadly, it is group based, not APKBUILD based :(
> 
> Of course, guide and some rules would help for this.

Indeed, Lets continue in the ticket Nathanael created! 
https://gitlab.alpinelinux.org/alpine/aports/issues/10689
> 
>> Never the less, what triggered for me to address the mailing list however
>> was commit 57d39405ad74 ("community/mtd-utils: adhere to aports code
>> style"). As the subject already states, 'adhere to aports code style' is
>> mentioned, but as mentioned earlier, I cannot seem to find this
>> aforementioned code style. It should be obvious then of course, that when
>> asking people to adhere to a coding style, this style needed to be
>> documented (and more importantantly rationalized). It shouldn't needed to be
>> mentioned, by having a guide (that can be checked with linters/checkers)
>> makes life easier, for both developers and reviewers (passes automated code
>> checks, no need to argue about style anymore).
>>
>> So in short, where is the style guide that everybody keeps talking about :)
>>
>> Thank you,
>>
>> Olliver

Hey Nathanael,

On 23-07-2019 21:29, Natanael Copa wrote:
> On Tue, 23 Jul 2019 08:28:11 +0200
> Olliver Schinagl <oliver+list@schinagl.nl> wrote:
> 
>> Hey list,
>>
>> over the past year or so, I've started to contribute to Alpine Linux's
>> aports, and have met various codestyles. To get to the bottom of what is
>> 'the' Alpine Linux codestyle, I came up empty handed, there is no
>> CODESTYLE.md, no search query that finds anything nor on the wiki.
>>
>> Now of course, the document could be hiding out of plain site, and if
>> that is the case, other then saying 'make it findable', I appologize for
>> even bringing anything up here.
> 
>> As I was about to do a version bump to community/mtd-utils, I noticed
>> a lot of activity on the package, a great thing!. Sadly I came to the
>> realization, that in the 'default' aports workflow, the
>> maintainers/contributers do not get CC-ed on changes. As I'm mostly
>> only familiar with Linux, U-Boot and busybox development, I missed
>> the 'get_maintainers' step, where people involved in abuild's would
>> be kept in the loop.
> 
> This can and be improved.
Excellent, what did you have in mind? I suspsect a CONTRIBUTING.md would 
fix this.

* Read codestyle
* always include maintainers in the CC/invite to review.

For linux (and u-boot) the get_maintainers script goes over the git log 
of a APKBUILD and gets the contributers.

Actually, this can be easily automated.
Codestyle -> linting via the CI.
maintainers can also be found using the CI (parse the APKBUILD file, 
include maintainer via the gitlab API

Getting the auther authors/contributers from the log however can cause 
quite some false positives, e.g. if someone goes over the entire 
repository and fixes some global issue (removes all cd $builddir 
statements) he now gets flagged for each MR.

> 
>> Never the less, what triggered for me to address the mailing list
>> however was commit 57d39405ad74 ("community/mtd-utils: adhere to
>> aports code style"). As the subject already states, 'adhere to aports
>> code style' is mentioned, but as mentioned earlier, I cannot seem to
>> find this aforementioned code style. It should be obvious then of
>> course, that when asking people to adhere to a coding style, this
>> style needed to be documented (and more importantantly rationalized).
>> It shouldn't needed to be mentioned, by having a guide (that can be
>> checked with linters/checkers) makes life easier, for both developers
>> and reviewers (passes automated code checks, no need to argue about
>> style anymore).
>>
>> So in short, where is the style guide that everybody keeps talking
>> about :)
> 
> Its still unwritten, sorry. I guess its based on what is common for the
> majority of the APKBUILDs. It is ofcourse not realistic that everyone
> read all of the thousands of APKBUILDs to get how things are normally
> done, so yeah, this should be fixed.
> 
> I created an issue: https://gitlab.alpinelinux.org/alpine/aports/issues/10689
Created a MR to kick off the ball.

> 
>>
>> Thank you,
>>
>> Olliver
> 

Just a very short follow up,

On 24-07-2019 09:13, Olliver Schinagl wrote:
> Hey Nathanael,
> 
> On 23-07-2019 21:29, Natanael Copa wrote:
>> On Tue, 23 Jul 2019 08:28:11 +0200
>> Olliver Schinagl <oliver+list@schinagl.nl> wrote:
>>
>>> Hey list,
>>>
>>> over the past year or so, I've started to contribute to Alpine Linux's
>>> aports, and have met various codestyles. To get to the bottom of what is
>>> 'the' Alpine Linux codestyle, I came up empty handed, there is no
>>> CODESTYLE.md, no search query that finds anything nor on the wiki.
>>>
>>> Now of course, the document could be hiding out of plain site, and if
>>> that is the case, other then saying 'make it findable', I appologize for
>>> even bringing anything up here.
>>
>>> As I was about to do a version bump to community/mtd-utils, I noticed
>>> a lot of activity on the package, a great thing!. Sadly I came to the
>>> realization, that in the 'default' aports workflow, the
>>> maintainers/contributers do not get CC-ed on changes. As I'm mostly
>>> only familiar with Linux, U-Boot and busybox development, I missed
>>> the 'get_maintainers' step, where people involved in abuild's would
>>> be kept in the loop.
>>
>> This can and be improved.
> Excellent, what did you have in mind? I suspsect a CONTRIBUTING.md would 
> fix this.
> 
> * Read codestyle
> * always include maintainers in the CC/invite to review.
> 
> For linux (and u-boot) the get_maintainers script goes over the git log 
> of a APKBUILD and gets the contributers.
> 
> Actually, this can be easily automated.
> Codestyle -> linting via the CI.
> maintainers can also be found using the CI (parse the APKBUILD file, 
> include maintainer via the gitlab API
Actually, the feature is called CODEOWNERS and is a text file [0].
(It may need an upgrade of the gitlab instance ;) )

Sadly however, it is not sourced from the source directory, but a global 
directory (this probably will change in the future)

I would recommend in that case, either maintainers add themselves and 
the projects they maintain to the CODEOWNERS file, or automatically have 
a bot-commit on each maintainer 'field' change of the APKBUILD file.

Olliver

[0] https://docs.gitlab.com/ee/user/project/code_owners.html

> 
> Getting the auther authors/contributers from the log however can cause 
> quite some false positives, e.g. if someone goes over the entire 
> repository and fixes some global issue (removes all cd $builddir 
> statements) he now gets flagged for each MR.
> 
>>
>>> Never the less, what triggered for me to address the mailing list
>>> however was commit 57d39405ad74 ("community/mtd-utils: adhere to
>>> aports code style"). As the subject already states, 'adhere to aports
>>> code style' is mentioned, but as mentioned earlier, I cannot seem to
>>> find this aforementioned code style. It should be obvious then of
>>> course, that when asking people to adhere to a coding style, this
>>> style needed to be documented (and more importantantly rationalized).
>>> It shouldn't needed to be mentioned, by having a guide (that can be
>>> checked with linters/checkers) makes life easier, for both developers
>>> and reviewers (passes automated code checks, no need to argue about
>>> style anymore).
>>>
>>> So in short, where is the style guide that everybody keeps talking
>>> about :)
>>
>> Its still unwritten, sorry. I guess its based on what is common for the
>> majority of the APKBUILDs. It is ofcourse not realistic that everyone
>> read all of the thousands of APKBUILDs to get how things are normally
>> done, so yeah, this should be fixed.
>>
>> I created an issue: 
>> https://gitlab.alpinelinux.org/alpine/aports/issues/10689
> Created a MR to kick off the ball.
> 
>>
>>>
>>> Thank you,
>>>
>>> Olliver
>>

On Wed, 24 Jul 2019 09:13:39 +0200
Olliver Schinagl <oliver+list@schinagl.nl> wrote:

> Hey Nathanael,

Hi!

 
> On 23-07-2019 21:29, Natanael Copa wrote:
> > On Tue, 23 Jul 2019 08:28:11 +0200
> > Olliver Schinagl <oliver+list@schinagl.nl> wrote:
> >   
> >> Hey list,
> >>
> >> over the past year or so, I've started to contribute to Alpine Linux's
> >> aports, and have met various codestyles. To get to the bottom of what is
> >> 'the' Alpine Linux codestyle, I came up empty handed, there is no
> >> CODESTYLE.md, no search query that finds anything nor on the wiki.
> >>
> >> Now of course, the document could be hiding out of plain site, and if
> >> that is the case, other then saying 'make it findable', I appologize for
> >> even bringing anything up here.  
> >   
> >> As I was about to do a version bump to community/mtd-utils, I noticed
> >> a lot of activity on the package, a great thing!. Sadly I came to the
> >> realization, that in the 'default' aports workflow, the
> >> maintainers/contributers do not get CC-ed on changes. As I'm mostly
> >> only familiar with Linux, U-Boot and busybox development, I missed
> >> the 'get_maintainers' step, where people involved in abuild's would
> >> be kept in the loop.  
> > 
> > This can and be improved.  
> Excellent, what did you have in mind? 

I was wondering how to do it in practice given that some people send
patches via email, and some via github. We are also moving to gitlab
and will hopefully be able to disable github PRs.

In any case, it should be automated as much as possible.

> I suspsect a CONTRIBUTING.md would 
> fix this.
> 
> * Read codestyle
> * always include maintainers in the CC/invite to review.

Yes. Something like that would be great. We already have
a .github/CONTRIBUTING.md, but it is specific for github.

> For linux (and u-boot) the get_maintainers script goes over the git log 
> of a APKBUILD and gets the contributers.
> 
> Actually, this can be easily automated.
> Codestyle -> linting via the CI.
> maintainers can also be found using the CI (parse the APKBUILD file, 
> include maintainer via the gitlab API

Something like that would be great.

> Getting the auther authors/contributers from the log however can cause 
> quite some false positives, e.g. if someone goes over the entire 
> repository and fixes some global issue (removes all cd $builddir 
> statements) he now gets flagged for each MR.

Some users add them selves as `# Contributor: ` in the APKBUILD. I
guess it should be enough to notify the listed maintainer and
contributors. That way it is possible to disable notifications in case
someone would want to do that.


> > I created an issue: https://gitlab.alpinelinux.org/alpine/aports/issues/10689  
> Created a MR to kick off the ball.

Awesome! Thank you!

Turns out to be our first MR in gitlab :) Congrats!

-nc