Ok, looks like I figured out how to use Eclipse and Git. Steve's videos are still correct.
I also figured out the problem with #881(#880). It's a simple fix. Where would I put the change? Should it be under "origin/branches/impresscms_1.3"?
Ticket #880 looks easy (maybe I'm wrong).
David, I think i need to first get familiar with checking out the source using Eclipse. I wonder if some of the team uses PHPStorm or some other/better IDE than Eclipse?
I just downloaded Eclipse Neon, and have to figure out my Git setup. Not sure if EGit is needed or not? I looked at Steve's videos on YouTube, but I think Eclipse may have changed a bit since then.
For tickets to work on, should I normally select "Current Milestone". I'm familiar with sprints using JIRA, but I see that Assembla is a bit similar. Any suggestions or guides would be helpful for me.
Hi Alvin, I just invited you to the project on Assembla. Let me know if I can help you with a walkthrough.
What do you want to work on at first?
Glad to see that you want to help out with development, we can always use more contributors and fresh views on how to do what we do.
We are using Assembla for their ticketing tool, which is one of the best solutions around, and we have migrated our Sourcecode to Github recently.
I see now that our landing page on assembla is broken, I've created a ticket with them to look into it. That way, you can create your account.
In the meantime, you can already find the sourcecode on github : https://www.github.com/impresscms/impresscms
I'm interested in doing some development work either for ImpressCMS or Finalize. The main purpose of this would be to work on a project for my degree that I'm trying to finish.
I wanted to get familiar with the source code etc... and essentially get up to speed on the software itself.
I've read that you use Assembla, but I can't figure out how to request to join directly.
This is being looked at in this ticket : https://www.assembla.com/spaces/impresscms/tickets/431-improve-use-of-stopspammer---reduce-false-positives-and-make-it-optional
Yesterday I asked a Chinese colleague to register on a work site, but he was getting an error that "this IP address is not allowed to register".
I thought it might be Protector, but after clearing the logs he was still stuck. Then I had a vague recollection about some anti-spam checking and found this thread. I disabled the IP check and he was ok.
I don't usually allow public registrations to my work sites anymore so it's not a big issue for me. But I thought I should flag it here as it is a useful example.
In this case, the guy was behind a (probably huge) Chinese academic network. It's pretty much guaranteed that the network would be infested with spambots, malware and you name it, as is typical in this part of the world. But at the same time, it's a real network full of people we need to interact with.
So, I guess I'm saying we need to be mindful of the opportunity for false positives with IP-based approaches.
[Random story time]
I had a similar problem a few years back with my server's firewall. I was getting frequent reports from developing country people that they couldn't access a site. I knew the site was working, but there the reports were too frequent to ignore.
It turned out that the firewall looked at IP addresses of incoming requests and matched them against a list of IP blocks that were known to be in service (at that time, there were still some IP blocks that hadn't been issued). A request from an IP address not in service was assumed to be forged and discarded.
The problem was that the list the firewall had was out of date - new blocks had been issued, mainly to developing countries, and the firewall was assuming these were forged requests. I eventually found there was a config file entry for periodically updating the list, changed a '0' to a '1' and problem solved. I probably spent 2 weeks looking for that, over a period of months, to change one character!
From what I have read, I think that is possible (you can install phpdoc there). But what todo once documentation was generated, I have no idea.
I see 4 possible options here:
1) to add generated files with phpdoc to latest commit (is a good idea todo automatic commits?)
2) updated some special git repository with generated files (I think it's best way in the long run. Because it would be possible to have old and new api documentation and also it's possible to see changes between commits. Bad thing about this - we still do not have any module that could fully to use this feature)
3) use ftp/sfp to upload to imprescms ftp (possible security issues)
4) trigger phpdoc generation by url. F.e.: http://api.impresscms.org/update.php?comkit_id=COMMIT_ID (I think it's good choice if we use VPS... otherwise it would not work)
This question is related more about libraries/icms contents. System module has such lines too but I don't think so that would be a good idea to remove from there before rewriting this module.
It appears that Scrutinizer does more than just code quality checks - I did not know that. And, Travis doesn't really do code quality checks.
I do know that Travis does builds, runs tests and integrates with Jenkins. @jegelstaff at FreeFormSolutions (developer of the Formulize module), has been using Travis and Jenkins for some time.
Code quality is certainly a good thing. I do want to improve our time to test and release. Packaging releases is something that would be nice to script (Phing was used when we were on Subversion, and @fiammybe is adapting for git).
Do either of them give us the ability to run PHPDoc?
My last comment was to put it into perspective. Before we decide to take something out, it is good to understand why it was added in the first place.
Is it effective? Not completely. It is a simple test that can easily be broken.
Is it efficient? I'm sure when lots of classes are loaded that include this line, it isn't. The initial intent was better security, not efficiency.
Is it necessary? For a normal load of ImpressCMS, no. It does not prevent errors from occurring, nor does it provide a function not found elsewhere. For a developer that doesn't have the proper includes for even the autoloader to work, yes, it is necessary.
Is it used consistently within the code? No, there are many files without this test.
Would I do something in every file that uses include, include_once, require, require_once, file_get_contents, fopen, uses URI parameters, or does anything with the filesystem? Yes. Definitely.
skenow, I can understand why these lines are in files with some inline functionality but not with one class per file. Because you can't create class instance without knowing what class is in that file. So, You can't execute code without knowing contents. But if you know contents of the file you can define ICMS_ROOT_PATH constant before inclusion. So I don't see any reason why these lines can improve security for classes.
The check is a basic one for security that has been used in other applications, too. I have also seen others check to be sure they are not called directly or are a remote file inclusion.
The test for ICMS_ROOT_PATH carries a lot less overhead than doing a referrer check.
I'm still working on unit tests -> https://github.com/MekDrop/impresscms/tree/%23887-PHPUnit-tests-for-icms-library ;) I hope to finish this week all of them.
Today I have a bit taked look into documentation of Scrutinizer ( https://scrutinizer-ci.com/docs/guides/install_custom_software )
and I found that is possible to install some custom software. So maybe Scrutinizer is enought for us?
Also Scrutinizer can generate artifacts. So it's possible to use this feature for generating builds -> https://scrutinizer-ci.com/docs/build/build_artifacts -> https://scrutinizer-ci.com/docs/troubleshooting/debugging_browser_based_tests
That's what I thought as well. If you don't like travis-ci, codeship.io has something similar, and I'm testing it at the moment for the builds of ImpressCMS. THe fact that there are no unit tests defined is a bit of a problem, but I'm testing it anyway
Personally, I don't think there is a reason to leave those lines in, as long as there is no security implication, as I mentioned on slack.
The autoloader that's there in the current alpha release is quite primitive : I just replaced the current autoloader class with the composer autoloader. There will be quite a bit of stuff that is done by composer that still has code in the core that does a double job. Removing that code will be a work for the next couple of releases.
In every ICMS library file we have line similar to this:
From what I can understand both tools are similar but not for same tasks.
Travis CI is more about unit testing and Scrutinizer is more about code quality. https://www.adayinthelifeof.nl/2013/11/20/external-code-coverage-with-travis-scrutinizer/
I don't have much expierence with both of them. Maybe I'm wrong.
Personally I like scrutinizer because of the code checks that they do out of the box. I haven't looked at Travis or Scrutinizer more than the basic config, so I migth be wrong though
Just putting out a request for comment on which tool to use for testing and integration with our repository. Github allows us to use either, or both, and will run the tests whenever there is a commit or a pull request.
For Travis to work, we need to add .travis.yml to the core and define the basic tests.
What experience have you had with these?