Tackling Github

Recreating this Github thread into Forums for a more thorough discussion to then any actions that surface from this then we can ACTION them out to the appropriate party or platform (Trello etc).

Original discussion raised by horus68 on Github click here to see the original post

[Suggestion] PR by Trust your own dog food

Issue / Suggestion for Trello

@samus-aran We all know that there are new intentions to merge PR faster, but its taking time to see real action. As you know many users even don’t care to create PR because the time involved.
A funny fact can be found looking into some stats on PR waiting to be merged: @salesagility do not believe on their own “dog food” as most PR created but not processed are form Salesagility devs!
I know there must be a system to revise everything but if this is not working even in house, how come it will work for users (like me) that are outside the system.
One user I feel bad for is shogunpol with 51 PR waiting in line! Nobody can keep motivation like that!
So lets tackle your inside issues and start merging your own PR!

Expected Behavior

Start “eating your own dog food” https://en.wikipedia.org/wiki/Eating_your_own_dog_food and make a fast approval path for PR created by SalesAgility own devs.
Them move on to community users and only then you can ask for community help.

Actual Behavior

From a total 244 PR waiting to be merged, here are the stats
(even if some are not for hotfix branch but for development/feature branches they also are in line for so long)

List A - Opened PR from SalesAgility devs

(this is an assumption of me as there is no list of in house devs and some of them probably not work anymore for Salesagility)

51 PR by @shogunpol

23 PR by @gymad

11 PR by @Dillon-Brown
https://github.com/salesagility/SuiteCRM/pulls/Dillon-Brown
(all new ones: keep on working… we are counting on your work!)

9 PR by @daniel-samson

8 PR by @sgaved

4 PR by @MikeyJC

3 PR by @samus-aran

2 PR by @mattlorimer

2 PR by @salesagility-davidthomson

List B - Opened PR by former Salesagility devs?

4 PR by @emcrobert

1 Like

@samus-aran you probably also want to paste the answer you gave in Github here.

I’ve posted, more than once, IMHO, the best possible solution, given the limitations on human hand testing resources.

  1. MAXIMIZE THE USE OF THE FREE STANDARDIZED AUTOMATED CODE CHECKING TOOLS BUILT INTO GITHUB. Enable all the automated add-ons in github for checking the code. Travis-CI, Scrutinizer-CI, CodeCoverage Hub, php-cs-checker, etc.
  2. ADD THE SUGARCRM 6.5 PHPUNIT TESTS TO THIS REPO.
  3. REQUIRE ALL PRs TO INCLUDE EITHER A PHPUNIT TEST FILE, OR AN UPDATE TO THE EXISTING PHPUNIT TEST FILE ASSOCIATED WITH THAT PARTICULAR PHP CODE FILE.
  4. AUTO-MERGE AFTER 14 DAYS OF NO ACTIVITY ON THE PR’s CONVERSATION THREAD.

Doing these will save humans of 90% of the manual testing job.

Result is, with the same scant human resources dedicated to PRs, 20X more PRs will be merged.

I completely understand that merging PRs is work and takes time. However, you have a pretty good community and letting PRs sit forever is definitely discouraging for people who are actually taking the time and try to contribute.

I think you guys should start building some trust with your community. From what I see there are members that take things very seriously. Maybe you come up with a list of people you allow to perform merges. Or if you don’t feel comfortable with this let some members perform the merges into a community branch and release community versions in between your general release. That way you can let the community verify if there are problems with it. That might actually go well with your current LTS plans. Limit the detailed testing to the LTS versions and take more PRs in for the versions in between.

The long list of very qualified PRs really makes the project look bad. Some of those have been sitting there for over two years. Some of those are probably irrelevant by now.

2 Likes

@samus-aran

  1. For each of the non-merged PRs, would you kindly provide the specific reason as to why you have not merged it.

  2. For the free automated testing tools on github, such as CodeReviewHub, Travis, PHPunit, and about five more, would you kindly provide a reason for each one of them, as to why you have not activated, or debugged (in the case of Travis and the Sugar 6.5 PHPunit tests), to make it work.

  3. For code coverage on soniqube, as it is a single digit percentage, far, far less than the mandatory minimum of 80%, would you provide an explanation to why you aren’t working to increase it to 80%, or if you are, what exactly are you doing to accomplish this.

@Chris001 / ChrisC? This my personal thoughts and opinions to your comments on Github, and how we should tackle the github issues and pull requests.


The first point:
Technically speaking, we all are responsible for changes in SuiteCRM.

It is your responsibility to make a pull request with the changes you want. When making a big change, you will need to provide a really good reason for make such a change, because we have to consider the impact it will have on the community (not just us engineers/developers). Your changes must at least pass our functional tests and you must sign the contributor license agreement. When at least two trusted members have reviewed your code, it is then it is up to a member who has write access to make the final review and the final decision.

It really helps when the community tests the changes in their different environments, as that provides a level of trust that your changes works. You should also consider the direction that the SuiteCRM project is heading in. Changes that do not respect the context of our roadmap are less likely to make it into the product. However, this depends on the changes you are making.


The second point:
It is a pleasure to see someone who is as enthusiastic about tooling as I am and who really cares about our code quality.

But before you go off and start creating pull requests for your favorite PHP tools. I would like to express my opinion on such tooling. I think it is important to contextualize these tools with the priorities of the SuiteCRM project.

The great advantage of sonarqube is that we can see how we are progressing between each release. It integrates with many Integrated Development Environments (IDE). So it makes it really easy to review changes before making a pull request. It also integrates with other processes like Travis CI.

So far we have fixed thousands of the issues which have been raised by sonarqube. We are continuing to fix the areas in which we are working on. I would like to encourage the community to join in and help us resolve these issues. But please keep your pull requests as small as possible. That way it will be much easier to review and test.

It is important to recognize that security issues and high priority issues that stop the end user from being able to use SuiteCRM, will always take precedence over the things like tooling, libraries, code quality etc.


Regarding php-cs-fixer and tools that auto correct your code. I personality find that these tools are not a guarantee of code quality. I personally don’t feel that we need to have php-cs-fixer integrated with Travis CI. php-cs-fixer is not 100% full proof. In my opinion, it is more important to create code that works and then to spend time cleaning the code up as required. I don’t like the idea of an automated tool breaking my changes after I have made a pull request. It is the developers responsibility to ensure their code is of a good standard of quality. I think we could address such issues in the code review process.

When contributing code; it should at least adhere to our coding guidelines. When you are making a small change in a method of a class, that shouldn’t mean that you have to then clean up the entire file. You should clean up as you go. Only changing the areas that you touch. If a community contributer chooses to clean up more than they have to, then that is a bonus.

Perhaps it would be better to use php-cs-fixer to make smaller manageable chunks of changes. Since php-cs-fixer supports rules. If we used the php-cs-fixer to tackle certain categories of issues in subset of directories, so that we can see the changes broken down into separate commits on git. I would prefer that these changes would go through the review process instead of 100% automation. We could add php-cs-fixer to the composer require-dev and then encourage contributers to use this tool on the area in which they are working on. This would could have the side effect of encouraging contributors to keep the rules up to date as we improve the quality process.

I believe it is more important to focus on a subset of issues and then break down the bigger problems into smaller issues. This is where the role of sonarqube comes in. The quality gates on this tools gives us a focus on the issues we need to tackle.

I have a favorite saying: “if you take many tiny specks of dust, they will eventually become a mountain”.

Over time these issues will be resolved by the community and the product team. SuiteCRM will become a better product as a result.


My suggestion:

As the project progresses, in the future, I believe we will need to require that those who are make changes also provide tests. I think we should consider moving to a testing framework like codeception with integration with Travis CI. We could move the existing function test into this framework it minimal friction. We could then implement functional tests, selenium tests and performance tests when we are adding features. We could add unit tests when we are cleaning up the code.

I would like to hear what the community thinks about my thoughts on this topic.

2 Likes

My opinion on tackling Github is simple and I’ll repeat it: merge 10 simple PR’s a week, every week, schedule one day of samus-aran time per week for this, in a few months you’ll be on top of things and SuiteCRM will be a lot less buggy.

A different strategy is needed for the more complex PR’s (for example: this PR should have been merged into the Beta, because it’s a good PR, and a Beta would give us some room for risk-taking).

About the rest. I too feel nervous about sweeping changes, and about automatic changes. So I would generally support Daniel in his comments about php-cs-fixer.

However, maybe this all boils down to something that is quite testable: if we have it make all the automatic fixes and install SuiteCRM, do we get a functioning system? Or does it become a mess?

  • if it is a mess and very difficult to get to working conditions, then Daniel is surely right and the partial fixes, manual review strategy has to prevail.

  • if it seems to work fine then I would say the test is inconclusive (because we didn’t really test the thousands of code-paths changed). Unless we can get this on a Beta that many people play with for an extended period of time.

But trying to not get us stalled, I came up with this. I think Chris is pushing for two very interesting things:

  1. greater test coverage and quality
  2. tools that change the code in lots of places

If we’re nervous about 2., we can surely still push for 1. After all, 1. is what will make us less nervous about 2.! If we had proper tests we would feel much safer touching the code in several places.

Finally, like Chris, I personally feel the need for better code quality and application robustness is far more critical to this project than any of the new UI features that SalesAgility have invested so much on in the past two years that I’ve been following this project more closely.

So, while I say thank you for that work, I humbly suggest that the setting of priorities has not been the best. Especially if we consider that if you think robustness is not priority number one for you, you should at least make sure it’s a priority number two that advances with time, not one that is almost stalled (at least, when compared to what it could - and should - be).

Thanks everyone for this nice, civilized discussion - I feel it’s being productive. :slight_smile:

I completely understand why you would like to have trusted members review PRs. However, I think it would be helpful to rethink who is a trusted member. It appears to me that this is limited to senior developers but it really doesn’t have to be. Other projects have trusted members who are less experienced developers. If someone makes a minor CSS or language file change it doesn’t take a senior dev to approve it. You only need someone who you trust about only approving PRs that they understand and reviewed carefully. By making your senior devs review every minor PR you are essentially wasting your most important assets.

2 Likes

Just some stats for this week:
23 PR merged in the last 2 days
249 to go (or more 22 days with the same pace!!)

If all weeks looks like this the backlog wouldn’t exist and the application would be a better one.

38 PR are already “accessed” so maybe they could be the next ones to be merged (if a senior dev joins in):
https://github.com/salesagility/SuiteCRM/pulls?q=is%3Aopen+is%3Apr+label%3AAssessed

@djsamson @samus-aran

Before I share my remarks on Tackling GitHub, I have a few questions for you to answer, you’ll help clear up confusion around this topic,

  1. Who at Sales Agility is responsible for overall MANAGING the Sales Agility people and processes on this open source project ??

  2. Who at Sales Agility is responsible for seeing to the MAINTENANCE, BUG FIXING, etc. ??

  3. Who at Sales Agility is responsible for the OVERHAUL, RENOVATION, UPDATE of the poor quality, 2004 amateur grade coding, riddled with security risks, in the app, to current modern PHP standards which are acceptably secure, much more crash proof, and for which it is far easier to develop modules?

4, Who at Sales Agility is responsible for assuring an ever improving and consistent high quality result from the AUTOMATED TESTING ??

Apologies for my hiatus – I’ll try and answer your queries and suggestions as clearly as I can and then I’ll discuss a little more at the bottom.

[quote=“ChrisC” post=46620][color=#ff0044]I’ve posted, more than once, IMHO, the best possible solution, given the limitations on human hand testing resources.

  1. MAXIMIZE THE USE OF THE FREE STANDARDIZED AUTOMATED CODE CHECKING TOOLS BUILT INTO GITHUB. Enable all the automated add-ons in github for checking the code. Travis-CI, Scrutinizer-CI, CodeCoverage Hub, php-cs-checker, etc.
  2. ADD THE SUGARCRM 6.5 PHPUNIT TESTS TO THIS REPO.
  3. REQUIRE ALL PRs TO INCLUDE EITHER A PHPUNIT TEST FILE, OR AN UPDATE TO THE EXISTING PHPUNIT TEST FILE ASSOCIATED WITH THAT PARTICULAR PHP CODE FILE.
  4. AUTO-MERGE AFTER 14 DAYS OF NO ACTIVITY ON THE PR’s CONVERSATION THREAD.

Doing these will save humans of 90% of the manual testing job.

Result is, with the same scant human resources dedicated to PRs, 20X more PRs will be merged.[/color][/quote]

Thanks Chris :slight_smile: we are all aware of your often raised opinion is of the current situation on the use (lack of) of tools – some we agree and some we do not (unless proven to be benefit al to us). But all very much valid raising them.

  1. Agreed – we are looking into these.
  2. Already looking into this.
  3. Agreed – Time though required to train all developers to conform to this. Cases were certain Prs don’t require a PHPUnit test is granted as well.
  4. No – we don’t agree on this. The assumption here is that everyone supplying a PR is an expert in SuiteCRM then no this will not happen.

[quote=“till” post=46857][color=#ff0044]I completely understand that merging PRs is work and takes time. However, you have a pretty good community and letting PRs sit forever is definitely discouraging for people who are actually taking the time and try to contribute.

I think you guys should start building some trust with your community. From what I see there are members that take things very seriously. Maybe you come up with a list of people you allow to perform merges. Or if you don’t feel comfortable with this let some members perform the merges into a community branch and release community versions in between your general release. That way you can let the community verify if there are problems with it. That might actually go well with your current LTS plans. Limit the detailed testing to the LTS versions and take more PRs in for the versions in between.

The long list of very qualified PRs really makes the project look bad. Some of those have been sitting there for over two years. Some of those are probably irrelevant by now.[/color][/quote]

Thanks Till. Yes totally agreed. The reason that PRs sit there is honestly only in the last 6 months has there been a driven focus on the community (or the beginning of) – so we already started off with one foot trailing behind. However, that’s an excuse and we fully accept that. Onto the trust part – yup definitely and that is what is happening. If you see in another thread (https://suitecrm.com/forum/suggestion-box/13863-community-reviewing-process-discussion) where we are providing the community the ability to pass the testing and they are free to review the code. In regards to the ability to provide community members the ability to merge, yeah, at the moment the ability to write will be in the hands of SalesAgility as we govern the overall vision and direction of the product and will do in the foreseeable future. However, we do already have a list of members were are aware of – we do have plans for them to assist us more closely if they are willing to :slight_smile: thoughts include joint authority over languages, documentation etc. With this needs processes in place to ensure at least something is followed and its set out appropriately and thus requiring time… but I’ll discuss this a little later on.

Regards to the general release SuiteCRM they will still carry on with the often minor releases (trying to make them 3-4 weeks apart). We find not many people test prior to a major release and so this will need to be looked at perhaps inviting those trusted members or do a volunteer recruitment run to assist with testing.

[quote=“ChrisC” post=46924][color=#ff0044]@samus-aran

  1. For each of the non-merged PRs, would you kindly provide the specific reason as to why you have not merged it.

  2. For the free automated testing tools on github, such as CodeReviewHub, Travis, PHPunit, and about five more, would you kindly provide a reason for each one of them, as to why you have not activated, or debugged (in the case of Travis and the Sugar 6.5 PHPunit tests), to make it work.

  3. For code coverage on soniqube, as it is a single digit percentage, far, far less than the mandatory minimum of 80%, would you provide an explanation to why you aren’t working to increase it to 80%, or if you are, what exactly are you doing to accomplish this.[/color][/quote]

[quote=“ChrisC” post=47593][color=#ff0044]@djsamson @samus-aran

Before I share my remarks on Tackling GitHub, I have a few questions for you to answer, you’ll help clear up confusion around this topic,

  1. Who at Sales Agility is responsible for overall MANAGING the Sales Agility people and processes on this open source project ??

  2. Who at Sales Agility is responsible for seeing to the MAINTENANCE, BUG FIXING, etc. ??

  3. Who at Sales Agility is responsible for the OVERHAUL, RENOVATION, UPDATE of the poor quality, 2004 amateur grade coding, riddled with security risks, in the app, to current modern PHP standards which are acceptably secure, much more crash proof, and for which it is far easier to develop modules?

4, Who at Sales Agility is responsible for assuring an ever improving and consistent high quality result from the AUTOMATED TESTING ??[/color][/quote]

Ok, I have combined both posts here because I can clearly see abit of a trend here. Daniel answered the first post pretty clearly so thanks Daniel :thumbsup:

Chris, are you seeking something in particular in your questions because I honestly don’t see the benefit of focusing on a particular person(s) at SalesAgility will accomplish anything – unless the goal is to blame which that is something we will not accept. Every single member at SalesAgility is responsible for the upkeep of the project. We all provide insight, we all provide code, we all provide ideas, processes and actions of what we feel is the best for the project. Now, from the tone of the post (and your many others) this isn’t good enough – alright fine, we know this, but demanding who does what will not resolve the issues.

So in a round about way, you should address any member of SalesAgility, but if you want to be specific you can address myself but other SalesAgility members can participate on answering as they have just as much responsibility in the matter :slight_smile: But I will make it clear we will not allow anyone’s’ frustrations to take an negative attitude towards particular members of the SalesAgility team or the SuiteCRM Community.

[quote=“pgr” post=47428][color=#ff0044]My opinion on tackling Github is simple and I’ll repeat it: merge 10 simple PR’s a week, every week, schedule one day of samus-aran time per week for this, in a few months you’ll be on top of things and SuiteCRM will be a lot less buggy.

A different strategy is needed for the more complex PR’s (for example: this PR should have been merged into the Beta, because it’s a good PR, and a Beta would give us some room for risk-taking).

About the rest. I too feel nervous about sweeping changes, and about automatic changes. So I would generally support Daniel in his comments about php-cs-fixer.

However, maybe this all boils down to something that is quite testable: if we have it make all the automatic fixes and install SuiteCRM, do we get a functioning system? Or does it become a mess?

  • if it is a mess and very difficult to get to working conditions, then Daniel is surely right and the partial fixes, manual review strategy has to prevail.

  • if it seems to work fine then I would say the test is inconclusive (because we didn’t really test the thousands of code-paths changed). Unless we can get this on a Beta that many people play with for an extended period of time.

But trying to not get us stalled, I came up with this. I think Chris is pushing for two very interesting things:

  1. greater test coverage and quality
  2. tools that change the code in lots of places

If we’re nervous about 2., we can surely still push for 1. After all, 1. is what will make us less nervous about 2.! If we had proper tests we would feel much safer touching the code in several places.

Finally, like Chris, I personally feel the need for better code quality and application robustness is far more critical to this project than any of the new UI features that SalesAgility have invested so much on in the past two years that I’ve been following this project more closely.

So, while I say thank you for that work, I humbly suggest that the setting of priorities has not been the best. Especially if we consider that if you think robustness is not priority number one for you, you should at least make sure it’s a priority number two that advances with time, not one that is almost stalled (at least, when compared to what it could - and should - be).

Thanks everyone for this nice, civilized discussion - I feel it’s being productive. :-)[/color][/quote]

Thanks Pgr,

All very good points. Until we have the ability to easily label github (working on a bot as we speak) then a manual review we feel will prevail until there is a higher number of confident reviewers and automated testing in place.

10 PRS a day is definitely achievable when we get more people on board – this is what we are trying to do. As I’ve said it was only the senior developers doing the final review this will still be the case due to the nature of the application HOWEVER, where these senior developers come from can be anywhere i.e. partners, community members that have proven to be SuiteCRM devs etc. At the moment its only the internal team as:

  1. we don’t have a list of partners whom willing to assist (yet!)
    ---- We are drafting this up just now, and hopefully get processes in place to track and assist them
  2. don’t have a list of community members of that caliber.
    ---- More trickier one, perhaps similar to the Beta/RC process set up a recruitment run and have community based events i.e. hackaton :slight_smile:

Regards to the stability, Yes! We have increased our release cycle now to 6 months instead of the 3 months due to this – more time to tackle the bigger infrastructure issues and introduce testing/deployment and better development processes. So 7.10 roadmap will be published with a balance of new features as well as a githubathon (need to work on the name) to tackle the issues. Just to clarify also the fixes made during 7.10 development sprint will be applied to 7.8.x and 7.9.x. This has been scheduled at the beginning of 7.10 sprint (pretty much as soon as 7.9 is stable) so in the coming months (June/July etc).

I agreed in most parts yes, things like language changes, or documentation, or even certain bug fixes a senior member wouldn’t need to review it. Again in the other thread regarding updating the labels on github one suggestion was the Quick Fix which then community members can easily review and state its ready to go in.

With the bot another couple of labels we could introduce is ‘Needs Merged’ which the community can set and someone with write access can filter and quickly look and merge.

But with more responsibility we need to ensure that no one takes advantage – not that we are saying people will – but it does open up whom has write access. Do we perhaps have a separate branch specifically for languages, documentation, tests? That then merge into hotfix – a merging nightmare… perhaps?

The No part I would say that we haven’t got a lot of senior devs at our disposal and when they increase the PR will go more smoothly. Lots of little things make the jigsaw complete XD

Yes, there has been abit of flurrly of activity again, but we need to schedule time in (the product team are pretty fully booked as it is) to review but you can see daniel, gyula, dillon and myself have been able to provide insight via reviews. However ONE thing I will note from my personal experience is that Prs are still not at a quality that we can auto-merge at all even at the ‘Assessed’ label. I spend my time actually testing and providing better ways to implement. That is something we need to track to see how much effort each PR takes for anyone to review – the back and forth and the initial quality of code (I mean the implementation of the code in the SuiteCRM application!) - and WHY this is. Does SalesAgility need to make more effort to train the community how the application works? Documentation on the ins and outs of the system? A more defined github process (which we are working on!). Regards to automation tools would definitely help, but I was speaking more of the actual framework of the application and how the Prs don’t quite get where to best implement them.

Ok, I believe I have answered most of your questions and responses. I, again, thank you all for your input and lets try and establish an action list to proceed. I have placed attributes beside them to indicate Quick Wins

Action List:

  1. Get More Senior Developers
    ++++ In Process. Getting in contact with Partners & involving the Senior Developers from the Product Team.
    ++++ Arrange a Community campaign (whether this be a forum post or something more) to encourage more users to help with reviews (Quick Win)

  2. Set Up Github Labeling and Review Process
    ++++ Finalise the Process (see other thread) & Publish it (Quick Win)
    ++++ Set Up Github Bot to automate Label handling (Quick Win)

  3. Pull in SugarCRM 6.5 tests
    ++++ Set up new branch from develop to test
    ++++ Invite Developers to test/assess/review

  4. Schedule a dedicated sprint for Github Prs & Issues
    ++++ Scheduled during development sprint of 7.10 (Quick Win)
    ++++ PR will be tackled first and appropriately placed (develop/hotfix) (Quick Win)
    ++++ Issues will be tackled as an ongoing task throughout Release cycles
    ++++ Roadmap 7.10 to be published (including KPI targets) (Quick Win)

  5. Train Developers in PHPUnit Testing.
    ++++ This will need to be done in phases from simplest to more complex
    ++++ Train internally
    ++++ Establish Feasible Process for Internal, Community & Partners
    ++++ Establish Troubleshooting and Issues raised from Tests
    ++++ Documentation of the new Process & Tools

  6. Introduce Tools
    ++++ A phased approach over 7.10
    ++++ Establish which tools we are interested in using and Publish them in order of review/implementation (Quick Win)
    ++++ Create dedicated branches to make public
    ++++ Test Internally
    ++++ Invite Developers to test/assess/review

  7. Establish a continuous review of Github Issues & PRS
    ++++ Weekly Publishing of KPIs of Github issues/Prs (Quick Win)
    ++++ Assess Reviewing time for Partners & Community – this be monthly meetings with community etc

  8. Establish a more focused Community Group targeting certain areas (language, documentation, development, testing)
    ++++ Think this will need to be done in phases i.e. get one team going rather than all 5 or 6.
    ++++ Team roles and responsibilities (and whom to be involved)
    ++++ Communication process to be established between teams
    ++++ Management process of said teams

Ok, If you think there is more actions to be included or areas that the Community can suggest to progress more on then please shout out. Felt that the tooling isn’t a quick win as we need to ensure everyone is trained up and the time required to actual deal with the results of said tooling will come about (failures, incorrect formatting etc). As I’ve said earlier that the product team are pretty fully booked so Quick Wins are really a couple of weeks rather than days realistically. Its all about balance to keep the project going with SalesAgility and the Community :thumbsup:

2 Likes

@samus-aran

Thanks Ashley, that’s a great question. What’s the purpose of having one person be ultimately the responsible person for a certain area of an open source app development project - namely, managing people/processes, bugfix/maintenance, overhaul/upgrades, automated testing ?

The purpose of designating one ultimately responsible person for these four areas, is to assure that these highly important tasks get done promptly and are moving forward every day, without fail, and all of this, out in the open publicly on github.

It all comes back to human nature, and the fact that there are real people inside the organization which owns this project. Sales Agility has a good size group of employees right now. It’s a for-profit company, and you’re telling me all that employees are “responsible” for all areas of this open source app, Consequently, we’ve ended up getting what we have here, which is, more than 2 years (!!!) delay on reviewing and taking in about 200 very good PRs which fix bugs, improve performance, cleanly modernize the ancient 13-year-old dusty framework, and reduce security risks. Why should you, or any employee, you take your scarce time and contribute toward the technical task merging PRs into the app on github, and if you make a mistake you can naturally fear that it could come back to haunt you, when instead, you could do any number of other less risky, nicer and funner things with paying client projects and earning kudos for doing so, etc. Besides, working on the app on github isn’t the primary role of your job description anyway! So, nobody will expect you to truly consistently evolve the app every day, which is the level of effort it needs right now.

Sorry if you don’t like how this sounds, what we’re talking about and confronting here is human nature, and there’s no escaping it, we’re all human.

So, how do we take human nature into account in our process, for the purpose not letting this vital work get fuzzed out off to the sidelines for two years where it became “lost in committee” of the entire group of employees, none of whom is truly primarily responsible?

Simple solution. Try this out. Seriously. For 30 days, designate four employees’ primary job responsibilities to be each of the four areas outlined above. What have you got to lose? Nothing. They can split their time between, 4-6 hours/day on whatever they’re usually doing, and 2-4 hours/day on this primary responsibility. After 30 days, measure performance of the app project. If it didn’t work out, no big deal, change. If it does work out well, and it really ought to, if you do it right, great! Continue!

Why we must start running Automated Tests as soon as possible.

For comparison, let’s take a look at a very similar size and scope github PHP project - Joomla.
All should be familiar with Joomla. It’s the content management system used to host the SuiteCRM Customer Portal app.

  • Joomla is a web app,
  • It’s roughly the same size in terms of lines of code, classes, javascript libraries, use of frameworks and third party libraries.
  • It was started approximately the same year as SugarCRM (2004) so it had the same legacy remnants of PHP4 and early 5.x.
  • Joomla app has very similar functions to SugarCRM/SuiteCRM - users log in, build a profile, interact with one another, create content, read content, edit and delete content data. Admins can install third party modules, including, themes, mass emailing, workflow, newsletters, connect to social networks, etc. Almost all foreign languages are available as installable translations on Crowdin. Runs on Windows and Linux. Supports MySQL and MS SQL plus others. Very similar to Suite.

What does the Joomla PHP github project use for automated code checking and testing, and dependency management?

  1. PHPunit for unit testing the PHP code works.
  2. Jasmine and Karma for testing the javascript code.
  3. Travis for running PHPunit, phpcs, php-cs-fixer,
  4. Appveyor for testing the app on Windows,
  5. Drone to run the Karma javascript tests, and to run phpcs and compare it to Travis phpcs.
  6. PHP_CS for checking PHP code quality.
  7. php-cs-fixer for detecting bad things like duplicate code, fixing the format of the code so it matches coding standards ,etc.
  8. Composer, for automatic third party library dependency installs and security updates, and to autoload classes in accordance with current PSR standards.

Comments?

1 Like

[quote=“ChrisC” post=47646][color=#ff0044]

Thanks Ashley, that’s a great question. What’s the purpose of having one person be ultimately the responsible person for a certain area of an open source app development project - namely, managing people/processes, bugfix/maintenance, overhaul/upgrades, automated testing ?

The purpose of designating one ultimately responsible person for these four areas, is to assure that these highly important tasks get done promptly and are moving forward every day, without fail, and all of this, out in the open publicly on github.

It all comes back to human nature, and the fact that there are real people inside the organization which owns this project. Sales Agility has a good size group of employees right now. It’s a for-profit company, and you’re telling me all that employees are “responsible” for all areas of this open source app, Consequently, we’ve ended up getting what we have here, which is, more than 2 years (!!!) delay on reviewing and taking in about 200 very good PRs which fix bugs, improve performance, cleanly modernize the ancient 13-year-old dusty framework, and reduce security risks. Why should you, or any employee, you take your scarce time and contribute toward the technical task merging PRs into the app on github, and if you make a mistake you can naturally fear that it could come back to haunt you, when instead, you could do any number of other less risky, nicer and funner things with paying client projects and earning kudos for doing so, etc. Besides, working on the app on github isn’t the primary role of your job description anyway! So, nobody will expect you to truly consistently evolve the app every day, which is the level of effort it needs right now.

Sorry if you don’t like how this sounds, what we’re talking about and confronting here is human nature, and there’s no escaping it, we’re all human.

So, how do we take human nature into account in our process, for the purpose not letting this vital work get fuzzed out off to the sidelines for two years where it became “lost in committee” of the entire group of employees, none of whom is truly primarily responsible?

Simple solution. Try this out. Seriously. For 30 days, designate four employees’ primary job responsibilities to be each of the four areas outlined above. What have you got to lose? Nothing. They can split their time between, 4-6 hours/day on whatever they’re usually doing, and 2-4 hours/day on this primary responsibility. After 30 days, measure performance of the app project. If it didn’t work out, no big deal, change. If it does work out well, and it really ought to, if you do it right, great! Continue![/color][/quote]

Heh, you misinterpreted my post but anyway, lets start again then Chris. My point was really that there will be no blame game played in the SuiteCRM Team or Community and that is what I was making clear that you can address anyone in the team and we would respond collectively as we are all on the same page internally. Why I felt I needed to do so was from the attitude that your posts were portraying – seeking a figure head to target your frustrations – which I have already stated we will not allow :slight_smile:
But to address your above post – We are not a collective management organisation. Yes we do have certain members of management and have a product team. Discussions are made within the management team regarding the products vision, direction and everything you have previously mentioned and in regards to your suggestion of the dedicated time we have already scheduled that in (see my post) for 7.10.

However just to provide a realistic view of ourselves is that we aren’t a huge company and that we would have something to lose pulling people off projects for a month. We need a balance of revenue to maintain the product which I hope I have made it clearer that what we are doing and why we are here is because we lack resources (which I’m SURE I have stressed in other posts before) to do everything we want to do. Open Source is tough because of its nature, and if we had a donation which many big open source project get – lets use Drupal as an example – it was given funding of $500K from Acquia to the members of the Drupal community – AMAZING! And plus Drupal (& Joomla) for examples been around for years! We can certainly learn from them but they have many many years of development and resources than we do and will for a lone while yet.

Just to clarify regards to time difference between SuiteCRM and Joomla is that we took up SugarCRM from a later date i.e 2013 so we are still relatively new but I get your point. Yes the product is possible the same size but the community is ten times (term of phrase) larger, more diverse and more established than we are. I don’t think we would catch up with them over night. :slight_smile:
But, all in all I have set up the action list and we can establish that as a checklist in the weeks/months coming. We, SuiteCRM Team and the Community can achieve them with each other because we both need to – its all about resources and that is why we are here – to be realistic but progress as much as WE (BOTH SuiteCRM Team and the Community). Does that Action List assist (you Joomla tools review are noted and put with the tooling section)?

2 Likes

Quick reply to some of your points, Ashley.

  1. When it comes to being bluntly honest, especially on a github open source app development project, blunt honesty is something to be appreciated and to be grateful for. Because, when feedback is artificially good, the human tendency is, you just coast along, why change anything, all is good, and the average person may even stop checking for problems, under the philosophy of, “if it ain’t broke don’t fix it and don’t even look for any problems,” which is a huge mistake when it comes to software. It’s best to be honest and be able to receive total honesty. When there’s a problem with project itself, it’s best you be made aware of it as soon as possible, so that it can be fixed, as soon as possible. Same as a bug report for the software, best to get the bug report in sooner rather than later, for the soonest possible fix. I’m sure you agree.

Besides Greg, who are these certain members of management of whom you speak? It would be good of you to be transparent about who it is in your management, who are discussing and deciding on the vision, direction, etc., of this github application project, to which many of us have been contributing substantial amounts of our valuable time and efforts.

1 Like

@Chris, I think you are right about the need (and the advantages) of assigning clear responsibilities, but I don’t think you can safely assume that it’s not being done inside SalesAgility, or that you can demand the exposure of that in public.

I observe that SalesAgility keeps some things private, and quite frankly I think that’s only natural. I mean, think about it, tens of thousands of downloads, so many people involved, and everybody has a request or an opinion for SalesAgility to answer. Often requests are very noisy, very ignorant, as we see in these forums occasionally. And they have no reason to share with us information about their business, other than periodically reminding us that they need to keep that business running so they can support and develop SuiteCRM for us, for free.

I do realize you have some authority here - that which comes with having contributed significantly, and being a long-time member of this community, and having good technical knowledge, but you’re not the only one they have to deal with, and they have their borders, their defenses, which is only natural when you have to live under constant pressure from many directions. And they have their way of doing things, every company has a culture, has a way… so you might do things differently if you were there, but I know you will respect that there is an ocean between you, and that sometimes we find we don’t all do business the same way.

That said, I empathize with you that you are here longer than me, you are tired of being patient, you are disillusioned with kind talk and and you are pushing for a tougher kind of talk (although respectful enough), hoping to get results. But take a step back and just consider whether that’s going to be effective from this point on.

They have answered you, they have spoken internally and with the community, they decided on actions, now it’s time to give them some time. I don’t think things today are the same as they were a year ago, or a month ago, or even a week ago. A lot has been done (some of it in response to your stimulus), and much of what has been done was about process design, not just end-product, so we can expect a better flow in the future.

@Ashley I notice I asked for the merging of 10 PR’s a week, but you read 10 PR’s a day and said it’s a good objective! That’s really cool, although perhaps you made a mistake and meant 10 a week… although your personal record is 18 merges on a Saturday, as far as I remember :slight_smile:

I thought the change of release cycle to 6 months was a good move, sometimes the 3 month releases seemed a bit rushed to me. However I fear that the release cycle alone won’t be enough to direct more focus to stability and robustness. You wrote that it gives more space for that, and you are right, but unless I am very wrong (which might be the case!) I think there is also some lack of motivation. There is little glory in fixing flaky code that often (though not always) works, when compared to pushing out new features or new UI.

The problem here might be a slightly skewed vision within SalesAgility when compared to the common people implementing SuiteCRM. I mean, you are experts; you don’t use shared hostings; you always have an even greater expert to consult in the desk next to you; so you solve problems quickly and efficiently, and move on.

Meanwhile, the common guy out there is dealing with multiple shared hosting quirks, all of them different, comes to the forums and asks awkwardly, without knowing how to ask, gets few answers, wastes weeks getting basic tasks to work… and then falls into the next trap. I think we need to put ourselves in the shoes of these people and REALLY get a more solid product out there.

The “it just works” philosophy, to quote Chris when he is at his best pushing us to be better. :slight_smile:

Peace! And thanks.

The management team of the Joomla project:
https://www.joomla.org/about-joomla/the-project/leadership-team.html

The management team for SugarCRM
https://www.sugarcrm.com/about/leadership

And the management team of Apache, probably one of the most famous open source projects:
https://www.apache.org/foundation/members.html

And how about the Salesforce management team:
https://www.salesforce.com/company/leadership/executive-team/

The pattern is clear. To make an app truly successful, you need to provide transparency as to who’s the management team.

Sales Agility should be transparent with the community and tell us who are the people on the management team behind the SuiteCRM github open source app.