Skip to content
This repository has been archived by the owner on Jan 15, 2023. It is now read-only.

Initial review of the code #1

Closed
ikedas opened this issue Jun 5, 2022 · 4 comments
Closed

Initial review of the code #1

ikedas opened this issue Jun 5, 2022 · 4 comments

Comments

@ikedas
Copy link
Member

ikedas commented Jun 5, 2022

This article describes the process of initial code review on a contribution to the Sympa community.

What we will do

We will carry out the process by which people who read the code give their opinions and criticisms, and the author modifies the code accordingly.

Place of work

The code base should be placed in a separate project (repository) from the main project, sympa-community/sympa-rest.

See also "Preparation work" below.

Duration

At least six months is necessary. In addition, several months are needed to revise the code based on the results of the review.

Method

The review will be carried out in public. In practice, the GitHub issue page is used.

  • Any person with an opinion, critique or the other proposals submits an issue on the project,
  • Interested people discuss it,
  • The authors make the necessary changes to the code according to the discussion.
  • Once a consensus is reached, the issue is closed.

Preparation work

Prior to the review process, preparation work will be performed.

  1. Obvious fixes of bugs and so on in the main project (sympa-community/sympa):
    => Submitted as the issues with PR on the main project.
  2. Additions or extensions of Sympa's internal API (Sympa::Request::Hander modules) or the other functionalities added into the main project:
    => Submited as the issues (possiblly with PR) on the main project.
  3. Remainder, i.e. additions specific to REST API (modules under Sympa::API::REST, related entries in cpanfile and tests):
    => Put in the place of work above.

Preparation work is expected to take for one to two months.


After a week or so, we would like to start the preparation work.

@ikedas
Copy link
Member Author

ikedas commented Nov 25, 2022

Sorry for the delay. I finally had time to work on the preparation work.

Preparation work

First of all, I want to say one thing.

Sympa::API seems to wrap Sympa::Request, but I don't agree with this development approach. Because it increases complexity and reduces maintainability (the code of current Sympa is/was severely unmaintainable, largely due to similar approaches in the past). If you think there is a problem with current Sympa::Request, please suggest a solution first --- instead of leaving the problem and building another one.

If you don't have any suggestions, I'll try to find a solution.


The following are the requests for the preparation work I mentioned above.

1. Obvious fixes of bugs and so on in the main project (sympa-community/sympa)

@yent, please submit these things as the issues with PR on the main project.


  • commit Renater/sympa@6b85c5a3a791c348910b28d036c72b42a36d36a3
    Author: etiennemeleard etienne.meleard@renater.fr
    Date: Wed Jun 29 09:47:42 2022 +0200

Propagate config sructural errors up to caller (does not change defaut behaviour), also only check for permission issues if any real changes, this allows for structural config error feedback in non-form based edition modes (rest ...)

src/lib/Sympa/Config.pm

  • commit Renater/sympa@f9770956430311e1bd2ecec9c4aca8e2d1b055dd
    Author: etiennemeleard etienne.meleard@renater.fr
    Date: Wed Dec 15 14:57:40 2021 +0100

Add message object support to viewmod handler

src/lib/Sympa/Request/Handler/viewmod.pm

  • commit Renater/sympa@8ae9080e787c501969a55f713e7e323fb65cda5f
    Author: etiennemeleard etienne.meleard@renater.fr
    Date: Fri Dec 10 11:47:17 2021 +0100

Improve posted message headers

src/lib/Sympa/Request/Handler/post_message.pm

  • commit Renater/sympa@5fbbdc85e8c66ed4a49a0c1d5ff0993a444eb26a
    Author: etiennemeleard etienne.meleard@renater.fr
    Date: Fri Dec 10 10:07:06 2021 +0100

Add a way for spindle/handlers to return results instead of shoving them the stash

src/lib/Sympa/Spindle.pm

  • commit Renater/sympa@15e56e70a90e04d84577f4bfc82d5820610ce22b
    Author: etiennemeleard etienne.meleard@renater.fr
    Date: Wed Dec 8 15:48:00 2021 +0100

Add processing exception for send, handler handles scenario result on its own

src/lib/Sympa/Spindle/AuthorizeRequest.pm

  • commit Renater/sympa@bb5e69174d2b25c484a26466cbbe14fe2b270575
    Author: etiennemeleard etienne.meleard@renater.fr
    Date: Wed Nov 10 17:15:10 2021 +0100

Fix var name

src/lib/Sympa/Spindle/ToAuth.pm

  • commit Renater/sympa@6a429306a8326cc950827a3b2905eae084251b8a
    Author: etiennemeleard etienne.meleard@renater.fr
    Date: Wed Nov 10 14:14:38 2021 +0100

Improve request_auth capabilities, add support for more recipient address variables, add a way to give more context info to the template parser, use all that for move_user, fix bad default scenario logic for move_user

src/lib/Sympa/Spindle/ToAuth.pm
src/lib/Sympa/Spindle/AuthorizeRequest.pm
default/scenari/move_user.auth
default/mail_tt2/request_auth.tt2

  • commit Renater/sympa@0c8d73b533771b17bf0b1159b77cc88e968db9a9
    Author: VERDIN David david.verdin@renater.fr
    Date: Thu Oct 21 16:53:54 2021 +0200

Making listname parameter mandatory.

src/lib/Sympa/Request/Handler/create_list.pm

  • commit Renater/sympa@b2485faab8dc0d63cd59a4432a58500ec5e4e54d
    Author: VERDIN David david.verdin@renater.fr
    Date: Thu Oct 21 16:37:19 2021 +0200

Fixing broken scenario.

default/scenari/add.closed


2. Additions or extensions of Sympa's internal API (Sympa::Request::Hander modules) or the other functionalities added into the main project:

@yent, these commits are a mix of changes to the main project and those that are not. The former should be submitted as issues or PRs in the main project. The latter are mentioned first.


  • commit Renater/sympa@04ade43eaab20105c036b0be628c7bd523511485
    Author: etiennemeleard etienne.meleard@renater.fr
    Date: Wed Mar 2 14:54:21 2022 +0100

Add pending messages handler and endpoint

src/lib/Sympa/Request/Handler/distribute.pm
src/lib/Sympa/API/REST/Handler/Pending/Messages.pm

  • commit Renater/sympa@95076dbf1460589aa90d18cde75fe7f8c6536d45
    Author: etiennemeleard etienne.meleard@renater.fr
    Date: Wed Dec 15 14:56:28 2021 +0100

Add pending message rejection endpoint with blocklist and spam reporting support, add tests

t/rest/reject.t
t/data/report_spam.pl
t/create_test_environment.pm
src/lib/Sympa/Request/Handler/report_spam.pm
src/lib/Sympa/Request/Handler/reject.pm
src/lib/Sympa/Request/Handler/add_to_blocklist.pm
src/lib/Sympa/API/REST/Handler/Pending/Messages.pm
src/lib/Sympa/API.pm
src/lib/Makefile.am

  • commit Renater/sympa@5fbbf1e182d0b9defeba8ca7bae18bb3d3cc5647
    Author: etiennemeleard etienne.meleard@renater.fr
    Date: Fri Dec 10 17:22:03 2021 +0100

Add pending message getter endpoint and related tests

t/rest/get_pending_message.t
src/lib/Sympa/Request/Handler/viewmod.pm
src/lib/Sympa/Archive.pm
src/lib/Sympa/API/REST/Handler/Pending/Messages.pm
src/lib/Sympa/API.pm

  • commit Renater/sympa@890af0e8ae8f49633111cab799c90df06c025bfb
    Author: etiennemeleard etienne.meleard@renater.fr
    Date: Fri Dec 10 11:49:33 2021 +0100

Add pending messages getter endpoint, related openapi desc and tests

t/rest/get_pending_messages.t
src/lib/Sympa/Request/Handler/modindex.pm
src/lib/Sympa/API/REST/Handler/Pending/Messages.pm
src/lib/Sympa/API.pm

  • commit Renater/sympa@c94efd33ce2246e905ab08f7db093f18aa085827
    Author: etiennemeleard etienne.meleard@renater.fr
    Date: Fri Dec 10 10:08:34 2021 +0100

Use new spindle result handling feature to simplify api interface, also get rid of rest specific status outputs in api

src/lib/Sympa/Request/Handler/post_message.pm
src/lib/Sympa/API/REST/Handler/User.pm
src/lib/Sympa/API/REST/Handler/List/Subscribers.pm
src/lib/Sympa/API/REST/Handler/List.pm
src/lib/Sympa/API.pm

  • commit Renater/sympa@6a10a1022dd6e2683f08efc4841adf34cd1f42c2
    Author: etiennemeleard etienne.meleard@renater.fr
    Date: Wed Dec 8 16:50:50 2021 +0100

Add multipart message posting support

t/rest/post_message.t
t/create_test_environment.pm
src/lib/Sympa/Request/Handler/post_message.pm
src/lib/Sympa/API/REST.pm
default/openapi.yaml

  • commit Renater/sympa@599d2db7a86f84da1305fdf6e86b5122599bffe7
    Author: etiennemeleard etienne.meleard@renater.fr
    Date: Wed Dec 8 15:49:22 2021 +0100

Add messgae posting endpoint, related tests and openapi spec

t/rest/post_message.t
src/lib/Sympa/Request/Handler/post_message.pm
src/lib/Sympa/API/REST.pm
src/lib/Sympa/API.pm
default/openapi.yaml


@ikedas
Copy link
Member Author

ikedas commented Dec 5, 2022

@yent, Could you please answer?

@ikedas
Copy link
Member Author

ikedas commented Dec 14, 2022

@yent, if we do not receive any response in one month, i.e. by December 25th, it may be assumed that you have no intention to contribute to the Sympa Community, and this repository will be closed.

@ikedas
Copy link
Member Author

ikedas commented Dec 26, 2022

No response. Close.

@ikedas ikedas closed this as not planned Won't fix, can't repro, duplicate, stale Dec 26, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant