Take a look to my fork :)

[tigerblue77]

Hi :)

I just forked your repository, could you take a look to my adds (I couldn't test it because I miss dependencies that are not documented and your paths in every file makes your code difficult to improve and I wouldn't change everything, just improve ^^...)

I'm sorry about a few things :

• if my English is not perfect, it's not my main language but I do my best
• if my code is not perfect : I didn't tested it but it should probably work :P

My objective is to include a module like this code on my personal web platform (that regroups some useful tools of all kinds)

Have a good day !

[kevinquinnyo]

@tigerblue77 I'll take a look later tonight if I can. Thanks!

[kevinquinnyo]

I didn't have much time to look at it, and to be honest I did not know about Synology Hybrid Raid (SHR) before today.

Is it correct to say that SHR is actually only applicable to RAID 1 or RAID 5?

And if so, should we extend those classes? kevinquinnyo\Raid\RaidOne and kevinquinnyo\Raid\RaidFive ?

Ex:

// Raid1SHR.php
namespace kevinquinnyo\Raid;

class Raid1SHR extends Raid1
{
}

// Raid5SHR.php
namespace kevinquinnyo\Raid;

class Raid5SHR extends Raid5
{
}

If you can help educate me a bit on this I'd love to help you land a pull request that supports it. I'll look into it on my own in the meantime. Let me know if I misunderstand -- I'm not exactly a hardware guy ;)

[tigerblue77]

Hi,

thank you for your answer and for looking at my code. By my side I'm an engineering apprentice in systems, networks and IT infrastructure but I code I my free time and learn it at school.

Yes i think you misunderstand, I would love to explain to you, but my explanations may not be very clear so I give your the Synology's explanation : https://www.synology.com/en-global/knowledgebase/DSM/tutorial/Storage/What_is_Synology_Hybrid_RAID_SHR

Don't hesitate to let me know if you don't understand it, I'll explain how the SHR RAID works. In fact, it is a succession of RAID 5's that permit to considerably reduce the lost space when you have drives of different sizes. As a result, I think that one classe is the most appropriate choice.

Could you improve your code a bit to remove the "namespace" and "use" directives at the beginning of each file please ?

If I'm not mistaken, your code depends on 2 libraries that (I think) you should include in this repository. I don't know what their use is, but I think if you use them, they are useful. It would be breat to have all we need in the repository.

Don't hesitate to tell me if I'm wrong as your code is very clear and comprehensible ; I don't want to ruin it.

Thanks,
Dylan

EDIT : - regrouping all dependancies in this repository will allow me to test my code easily :)
- to test you functions, including my SHR one, you can use the synology web RAID calculator that is really cool ! https://www.synology.com/en-global/support/RAID_calculator

[tigerblue77]

Hi, is it ok for you ? :)

[kevinquinnyo]

@tigerblue77 I'm really sorry I haven't had a chance to dig into this yet, but I definitely will.

In the meantime, I think there might be some confusion on how to use the library. You mentioned that you're having dependency issues? Did you run composer install? This library requires composer which you can download here.

[tigerblue77]

Hi,

Sorry for the delay. I don't know "Composer" at all and I didn't know I had to install it to make it work. I will try in a Debian VM to be safe and I'll tell you :)

[tigerblue77]

Hi,

sorry for the delay again, I taked a look to composer and installed it but I should have miss something as it don't work...
Here is what I've done :
`mkdir php-raid

cd php-raid

php -r "copy('https://getcomposer.org/installer', 'composer-setup.php');"

php -r "if (hash_file('sha384', 'composer-setup.php') === '48e3236262b34d30969dca3c37281b3b4bbe3221bda826ac6a9a62d6444cdb0dcd0615698a5cbe587c3f0fe57a54d8f5') { echo 'Installer verified'; } else { echo 'Installer corrupt'; unlink('composer-setup.php'); } echo PHP_EOL;"

php composer-setup.php

php -r "unlink('composer-setup.php');"

sudo mv composer.phar /usr/local/bin/composer

git clone https://github.com/kevinquinnyo/php-raid.git .

composer install`

But after that...

Where do I code my "Main" php file to test ?
How do I run it ?

And at the beginnin of each of your files you have "namespace kevinquinnyo\Raid;". Do I have to change it ? Thanks by advance

[kevinquinnyo]

@tigerblue77 typically with a library like this you would be requiring it as a dependency in your own code by creating a composer.json of your own and listing it in the require section.

If you are just wanting to experiment with the library you can simply create a file in the base of the project directory like this:

<?php
require __DIR__ . '/vendor/autoload.php'; // this is what you are missing to autoload all the namespaces

use kevinquinnyo\Raid\Raid\RaidFive;
use kevinquinnyo\Raid\Drive;

$drives = [
    new Drive(1024, 'ssd', 1),
    new Drive(1024, 'ssd', 2),
    new Drive(1024, 'ssd', 3),
];
$raidFive = new RaidFive($drives);

var_dump($raidFive);

[tigerblue77]

Hi ! :)

thanks to your good explanations, I succeeded to run the project and improve the raid SHR that is now working great :) (have a look ;))

I'll continue to improve your/the/our project ^^ you can merge if you want :)

[kevinquinnyo]

@tigerblue77 Nice work!

Can you open a pull request? You should see something like this when you do it:

There are a few things that will need to be changed but you can go ahead and open the PR now:

1. Replace Main.php with unit tests for the new SHR raid type. You can run tests with vendor/bin/phpunit from the main project directory. (composer install provides that). Look at the current structure of the tests and create a new RaidSHRTest.php like the other raid levels. Try to create a test for every public method.

2. Run vendor/bin/phpcs -a (also provided by composer) and fix any code style issues that appear until it doesn't report any errors. (this will complain about things like spacing/indentation issues, etc)

I may also have a few other nitpicks after that but it looks good to me at first glance.

[tigerblue77]

@kevinquinnyo thank you :)

Yes I'll do but... Wow... Composer is a good tool, thanks for your explanations but daaammnn... I prefer tabulations haha ! x)

Please have a look to the changes I made to your DriveTest.php file please. As in Drive.php I cast the size to an int (it seems logical and was creating problems with my SHR RAID), I've editted the test :)

It seems your RAID 5, 6 and 10 are not working, I'll investigate before pulling ;)

[kevinquinnyo]

:) Did you mean to close your pull request #34 ? I'll be glad to take a look at it and probably suggest some improvements once it's open.

[tigerblue77]

Sorry I'm not sure to understand your question as English is not my main language ^^
I closed the pull request just because I found problems on my code that are solved now.

I'm a beginner in GIT/GitHub so don't hesitate to tell me if I do shit :P
I'll open a new pull request :)

[kevinquinnyo]

@tigerblue77 No problem. Your english is fine.

You can just open the PR, and then push commits up to your branch, or you can fix whatever you need to and then submit the PR. It's really up to you.

I'll be glad to help if you have any problems. :)

[tigerblue77]

@kevinquinnyo thank you :)

Well, I resolved the problems but there's still a problem with RAID 10... I have to think about the algorithm ^^' so I'll have a look when I'll have time :)

Thank you :) the PR is here #35

[kevinquinnyo]

@tigerblue77 thank you! I'll review this and report back soon

[tigerblue77]

Hello @kevinquinnyo, any news ?