Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement Common Build System #332

Closed
wants to merge 1 commit into from
Closed

Implement Common Build System #332

wants to merge 1 commit into from

Conversation

grangeway
Copy link
Contributor

For a long while now, the build a developer can do,
and the build one can automate on travis-ci are too different things.

This is the start of a process to unify the build system, such that a
developer can run a local build before/without having to wait for
travis-ci or some other automation system to kick in.

The requirements for this to run at the moment is an installation of
either phing or composer.

Phing route:

Phing can be installed, if not already installed by running:

pear channel-discover pear.phing.info
pear install [--alldeps] phing/phing

going to the Mantis directory, and running phing then starts a build.

This build will download composer, download a standard version of
phpcodesniffer/phpunit, and it's own copy of phing and run a build based
on these versions. This ensures that local + travis etc versions are the
same

Composer Route:

php -r "readfile('https://getcomposer.org/installer');" | php
composer install

A build can be run at this point by calling:

php vendor/bin/phing build

Note: at the moment, I've specified 'build' as a target here, as the
default option is to run a target called "buildsystem" at the moment,
which downloads all the dependencies

travis-ci.org build has been modified to include the process described
above so that a developers build and the build system build converge
towards being identical.

At the moment a 'build' consists of the following:

a) doing a quick lint check on PHP files for parse errors
b) Running some popular code metrics type utilities: phploc,pdepend
c) running phpcs with a custom ruleset which tries to follow mantis's rules
d) running phpunit tests
e) generating some api documentation
(atm, via phpdox, although should also generated via phpdoc2)
f) spit our a zip/tarball package of the build

Full build time is with codesniffer rules etc is around 2.5-3 minutes locally

however, individual steps can be run:

For example:

phing phpcs will run codesniffer rules against code base.

phing phpunit will run php unit tests.

This is very much a first step - I suspect we will alter the tools,
add tools and commands etc, as we work out whats useful and what isn't.

For example, I've seen some phing build scripts that deal with creating
and merging between local branches/github PR's etc.

For a long while now, the build a developer can do,
and the build one can automate on travis-ci are too different things.

This is the start of a process to unify the build system, such that a
developer can run a local build *before*/*without* having to wait for
travis-ci or some other automation system to kick in.

The requirements for this to run at the moment is an installation of
either phing or composer.

Phing route:

Phing can be installed, if not already installed by running:

pear channel-discover pear.phing.info
pear install [--alldeps] phing/phing

going to the Mantis directory, and running phing then starts a build.

This build will download composer, download a standard version of
phpcodesniffer/phpunit, and it's own copy of phing and run a build based
on these versions. This ensures that local + travis etc versions are the
same

Composer Route:

php -r "readfile('https://getcomposer.org/installer');" | php
composer install

A build can be run at this point by calling:

php vendor/bin/phing build

Note: at the moment, I've specified 'build' as a target here, as the
default option is to run a target called "buildsystem" at the moment,
which downloads all the dependencies

travis-ci.org build has been modified to include the process described
above so that a developers build and the build system build converge
towards being identical.

At the moment a 'build' consists of the following:

a) doing a quick lint check on PHP files for parse errors
b) Running some popular code metrics type utilities: phploc,pdepend
c) running phpcs with a custom ruleset which tries to follow mantis's rules
d) running phpunit tests
e) generating some api documentation
(atm, via phpdox, although should also generated via phpdoc2)
f) spit our a zip/tarball package of the build

Full build time is with codesniffer rules etc is around 2.5-3 minutes locally

however, individual steps can be run:

For example:

phing phpcs will run codesniffer rules against code base.

phing phpunit will run php unit tests.

This is very much a first step - I suspect we will alter the tools,
add tools and commands etc, as we work out whats useful and what isn't.

For example, I've seen some phing build scripts that deal with creating
and merging between local branches/github PR's etc.
@vboctor
Copy link
Member

vboctor commented Sep 21, 2014

Does this build pass based on our existing codebase in master? Or will it drive churn to pass?

I'm in favor of having a build system that is used in Travis validation and when we create nightly builds and releases. So +1 for that. However, don't want to open up a ton of changes to pass some code sniffing criteria or something else. I'm hoping your earlier cleanup did the trick.

However, we can always start with few criteria in the build and add more as we are ready. So having the skeleton and making it richer over time makes sense.

"support": {
"issues": "https://www.mantisbt.org/bugs",
"forum": "https://www.mantisbt.org/support/",
"wiki": "https://www.mantisbt.org/",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dregad
Copy link
Member

dregad commented Sep 22, 2014

I agree with this change on principle, it's a good thing overall that will increase quality of our build process, but will need to test this seriously before giving a +1 for merge

@grangeway
Copy link
Contributor Author

"However, don't want to open up a ton of changes to pass some code sniffing criteria or something else"

Tbh, I actually agree with you there. I think the 'major' things are hopefully implemented in the code base.

There are a few ruleset failures against the codebase in the PHPCS ruleset that i've generated. However, personally, I think most of those are issues with the ruleset for obscure cases in the code, where we need to disable/fix the rule rather then change the code.

In terms of Mantis, the biggest "rule" for coding standards that I believe we currently break is in terms of line length. @dregad tends to comment on long lines, and tends to go towards the 80 default that is often thrown around. there's a different PR where we discussed 80/100/120 etc.

I've set my editor to 80 characters since that discussion the other day, and tbh, I'm slowly concluding i'd need to find an editor that allows multiple line lengths to be set - i.e. one at 80 and one at 120.

For the most part, 80 tends to be too short a width for us in code.

Equally, if we were to use 120, there's several lines where we go way past that in the code base.

We need to decide where we actually stand on this at some point - as if we are going to reject PR's that have long lines (between 80<>120) for comments etc, I'd like to add a rule to avoid this. Equally, I'm not sure how feasible keeping code to 80 in general is, for example:

  • @param integer $p_new_category_id New category id (to replace existing category).
    ==> 85 characters.

I don't personally believe the above is overly verbose :)

TLDR:
a) I don't plan to do any sweeps on coding layout of mantis unless someone else comes up with something important - having phpdoc, semi-standardised white space and variable names etc would seem a good point to be at.
b) I'd like to get to a point where our ruleset passes 100%, but for most part, I think this will be a case of fixing rules
c) I would like to see us add a new rule at some point in the future to highlight lines >120 characters so we can fix those, but I'd probably see that as something that's optional and we'd consider fixing as we work on fixing code in a file, rather then something that needs an immediate fix. Exceptions i'd make to that rule would pretty much be lines that don't fit on a modern sized monitor screen.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants