Subject: Re: Alpine Linux aports codestyle
From: Olliver Schinagl <oliver+list@schinagl.nl>
To: Natanael Copa <ncopa@alpinelinux.org>
Cc: alpine-devel@lists.alpinelinux.org
References: <1c2e99a3-4995-296d-b6e1-6fbad8261ab8@schinagl.nl>
 <20190723212959.316ab849@ncopa-desktop.copa.dup.pw>
 <ef30c218-fe9d-d46f-9907-575c7978fbd5@schinagl.nl>
Message-ID: <920f37ce-2a26-8ab4-7247-c4117e8b5a9e@schinagl.nl>
Date: Wed, 24 Jul 2019 09:36:49 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101
 Thunderbird/60.8.0
MIME-Version: 1.0
In-Reply-To: <ef30c218-fe9d-d46f-9907-575c7978fbd5@schinagl.nl>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit

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
>>