You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jmeter.apache.org by ra0077 <gi...@git.apache.org> on 2016/03/15 14:24:10 UTC

[GitHub] jmeter pull request: bug59153_CSVDataSetFilesExceptions

GitHub user ra0077 opened a pull request:

    https://github.com/apache/jmeter/pull/167

    bug59153_CSVDataSetFilesExceptions

    More clear message for CSV Data Set with 
    filename empty
    wrong filename (not readable, not exist)
    
    I don't have made formating to help reviewing
    
    If anybody is ok, I will format it
    
    Antonio

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/ra0077/jmeter bug59153_CSVDataSetFilesExceptions

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/jmeter/pull/167.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #167
    
----
commit 7314516850021a157497c1fc83b796a01a98b29a
Author: ra0077 <ra...@gmail.com>
Date:   2016-03-15T13:22:52Z

    bug59153_CSVDataSetFilesExceptions
    
    More clear message for CSV Data Set with 
    filename empty
    wrong filename (not readable, not exist)
    
    I don't have made formating to help reviewing
    
    If anybody is ok, I will format it
    
    Antonio

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] jmeter pull request: bug59153_CSVDataSetFilesExceptions

Posted by vlsi <gi...@git.apache.org>.
Github user vlsi commented on a diff in the pull request:

    https://github.com/apache/jmeter/pull/167#discussion_r56164700
  
    --- Diff: src/core/org/apache/jmeter/services/FileServer.java ---
    @@ -428,6 +429,9 @@ private BufferedReader createBufferedReader(FileEntry fileEntry) throws IOExcept
                 isr = new InputStreamReader(fis);
             }
             return new BufferedReader(isr);
    +        } else {
    +            throw new IllegalArgumentException("File "+ fileEntry.file.getName()+ " must exist and be readable");
    --- End diff --
    
    Please move `else` branch to the start of the method.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] jmeter pull request: bug59153_CSVDataSetFilesExceptions

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/jmeter/pull/167


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

Re: [GitHub] jmeter pull request: bug59153_CSVDataSetFilesExceptions

Posted by Antonio Gomes Rodrigues <ra...@gmail.com>.
Hi,

    New commit with formating and swap else branch

    Antonio

Cet e-mail a été envoyé depuis un ordinateur protégé par Avast.
www.avast.com
<https://www.avast.com/fr-fr/lp-esg-fav?utm_medium=email&utm_source=link&utm_campaign=sig-email&utm_content=webmail&utm_term=OA-2109-B>
<#DDB4FAA8-2DD7-40BB-A1B8-4E2AA1F9FDF2>

2016-03-15 14:54 GMT+01:00 ra0077 <gi...@git.apache.org>:

> Github user ra0077 commented on the pull request:
>
>     https://github.com/apache/jmeter/pull/167#issuecomment-196829738
>
>     Ok
>
>     I will wait other review or a go before modify my PR to avoid problem
> like in my previous PR about this bug
>
>     Antonio
>
>
> ---
> If your project is set up for it, you can reply to this email and have your
> reply appear on GitHub as well. If your project does not have this feature
> enabled and wishes so, or if the feature is enabled but not working, please
> contact infrastructure at infrastructure@apache.org or file a JIRA ticket
> with INFRA.
> ---
>

[GitHub] jmeter pull request: bug59153_CSVDataSetFilesExceptions

Posted by ra0077 <gi...@git.apache.org>.
Github user ra0077 commented on the pull request:

    https://github.com/apache/jmeter/pull/167#issuecomment-196829738
  
    Ok
    
    I will wait other review or a go before modify my PR to avoid problem like in my previous PR about this bug
    
    Antonio


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

Re: [GitHub] jmeter pull request: bug59153_CSVDataSetFilesExceptions

Posted by Antonio Gomes Rodrigues <ra...@gmail.com>.
Hi,

Thanks to the feedback

check exists() is not necessary

canRead

public boolean canRead()

Tests whether the application can read the file denoted by this abstract
pathname.
Returns:true if and only if the file specified by this abstract pathname
exists *and* can be read by the application; false otherwise


New commit with exists() removed and else removed


Antonio




Cet e-mail a été envoyé depuis un ordinateur protégé par Avast.
www.avast.com
<https://www.avast.com/fr-fr/lp-esg-fav?utm_medium=email&utm_source=link&utm_campaign=sig-email&utm_content=webmail&utm_term=OA-2109-B>
<#4184246894694592853_DDB4FAA8-2DD7-40BB-A1B8-4E2AA1F9FDF2>

2016-03-17 10:54 GMT+01:00 sebb <se...@gmail.com>:

> On 17 March 2016 at 07:39, vlsi <gi...@git.apache.org> wrote:
> > Github user vlsi commented on a diff in the pull request:
> >
> >     https://github.com/apache/jmeter/pull/167#discussion_r56465359
> >
> >     --- Diff: src/core/org/apache/jmeter/services/FileServer.java ---
> >     @@ -418,16 +418,20 @@ private BufferedReader getReader(String alias,
> boolean recycle, boolean firstLin
> >          }
> >
> >          private BufferedReader createBufferedReader(FileEntry
> fileEntry) throws IOException {
> >     -        FileInputStream fis = new FileInputStream(fileEntry.file);
> >     -        InputStreamReader isr = null;
> >     -        // If file encoding is specified, read using that encoding,
> otherwise use default platform encoding
> >     -        String charsetName = fileEntry.charSetEncoding;
> >     -        if(!JOrphanUtils.isBlank(charsetName)) {
> >     -            isr = new InputStreamReader(fis, charsetName);
> >     +        if (!fileEntry.file.exists() || !fileEntry.file.canRead()
> || !fileEntry.file.isFile()) {
>
> Surely there's no need to check exists() if you check canRead()?
>
> >     +            throw new IllegalArgumentException("File "+
> fileEntry.file.getName()+ " must exist and be readable");
> >              } else {
> >     --- End diff --
> >
> >     Please remove `else` and the following braces.
> >     It would avoid indenting the following code.
> >
> >
> > ---
> > If your project is set up for it, you can reply to this email and have
> your
> > reply appear on GitHub as well. If your project does not have this
> feature
> > enabled and wishes so, or if the feature is enabled but not working,
> please
> > contact infrastructure at infrastructure@apache.org or file a JIRA
> ticket
> > with INFRA.
> > ---
>

Re: [GitHub] jmeter pull request: bug59153_CSVDataSetFilesExceptions

Posted by sebb <se...@gmail.com>.
On 17 March 2016 at 07:39, vlsi <gi...@git.apache.org> wrote:
> Github user vlsi commented on a diff in the pull request:
>
>     https://github.com/apache/jmeter/pull/167#discussion_r56465359
>
>     --- Diff: src/core/org/apache/jmeter/services/FileServer.java ---
>     @@ -418,16 +418,20 @@ private BufferedReader getReader(String alias, boolean recycle, boolean firstLin
>          }
>
>          private BufferedReader createBufferedReader(FileEntry fileEntry) throws IOException {
>     -        FileInputStream fis = new FileInputStream(fileEntry.file);
>     -        InputStreamReader isr = null;
>     -        // If file encoding is specified, read using that encoding, otherwise use default platform encoding
>     -        String charsetName = fileEntry.charSetEncoding;
>     -        if(!JOrphanUtils.isBlank(charsetName)) {
>     -            isr = new InputStreamReader(fis, charsetName);
>     +        if (!fileEntry.file.exists() || !fileEntry.file.canRead() || !fileEntry.file.isFile()) {

Surely there's no need to check exists() if you check canRead()?

>     +            throw new IllegalArgumentException("File "+ fileEntry.file.getName()+ " must exist and be readable");
>              } else {
>     --- End diff --
>
>     Please remove `else` and the following braces.
>     It would avoid indenting the following code.
>
>
> ---
> If your project is set up for it, you can reply to this email and have your
> reply appear on GitHub as well. If your project does not have this feature
> enabled and wishes so, or if the feature is enabled but not working, please
> contact infrastructure at infrastructure@apache.org or file a JIRA ticket
> with INFRA.
> ---

[GitHub] jmeter pull request: bug59153_CSVDataSetFilesExceptions

Posted by vlsi <gi...@git.apache.org>.
Github user vlsi commented on a diff in the pull request:

    https://github.com/apache/jmeter/pull/167#discussion_r56465359
  
    --- Diff: src/core/org/apache/jmeter/services/FileServer.java ---
    @@ -418,16 +418,20 @@ private BufferedReader getReader(String alias, boolean recycle, boolean firstLin
         }
     
         private BufferedReader createBufferedReader(FileEntry fileEntry) throws IOException {
    -        FileInputStream fis = new FileInputStream(fileEntry.file);
    -        InputStreamReader isr = null;
    -        // If file encoding is specified, read using that encoding, otherwise use default platform encoding
    -        String charsetName = fileEntry.charSetEncoding;
    -        if(!JOrphanUtils.isBlank(charsetName)) {
    -            isr = new InputStreamReader(fis, charsetName);
    +        if (!fileEntry.file.exists() || !fileEntry.file.canRead() || !fileEntry.file.isFile()) {
    +            throw new IllegalArgumentException("File "+ fileEntry.file.getName()+ " must exist and be readable");
             } else {
    --- End diff --
    
    Please remove `else` and the following braces.
    It would avoid indenting the following code.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

Re: [GitHub] jmeter pull request: bug59153_CSVDataSetFilesExceptions

Posted by Antonio Gomes Rodrigues <ra...@gmail.com>.
Thank a lot

Cet e-mail a été envoyé depuis un ordinateur protégé par Avast.
www.avast.com
<https://www.avast.com/sig-email?utm_medium=email&utm_source=link&utm_campaign=sig-email&utm_content=webmail>
<#DDB4FAA8-2DD7-40BB-A1B8-4E2AA1F9FDF2>

2016-04-01 10:22 GMT+02:00 Vladimir Sitnikov <si...@gmail.com>:

> > Can you merge it in few days if there are no news from other commiter?
>
> sure.
>
> Vladimir
>

Re: [GitHub] jmeter pull request: bug59153_CSVDataSetFilesExceptions

Posted by Vladimir Sitnikov <si...@gmail.com>.
> Can you merge it in few days if there are no news from other commiter?

sure.

Vladimir

Re: [GitHub] jmeter pull request: bug59153_CSVDataSetFilesExceptions

Posted by Antonio Gomes Rodrigues <ra...@gmail.com>.
Hi Vladimir,

Thank to your quick feed back

Can you merge it in few days if there are no news from other commiter?
Antonio

Cet e-mail a été envoyé depuis un ordinateur protégé par Avast.
www.avast.com
<https://www.avast.com/sig-email?utm_medium=email&utm_source=link&utm_campaign=sig-email&utm_content=webmail>
<#DDB4FAA8-2DD7-40BB-A1B8-4E2AA1F9FDF2>

2016-03-31 23:48 GMT+02:00 Vladimir Sitnikov <si...@gmail.com>:

> Antonio, thanks for your patience.
>
> The change looks good to me.
>
> Vladimir
>

Re: [GitHub] jmeter pull request: bug59153_CSVDataSetFilesExceptions

Posted by Vladimir Sitnikov <si...@gmail.com>.
Antonio, thanks for your patience.

The change looks good to me.

Vladimir

Re: [GitHub] jmeter pull request: bug59153_CSVDataSetFilesExceptions

Posted by Antonio Gomes Rodrigues <ra...@gmail.com>.
Hi Vladimir and Sebb,

Do you think you can take some time to review my PR and merge it if it's ok
or said to me how to ameliorate it to be merged?

Thank
Antonio

2016-03-25 12:14 GMT+01:00 Antonio Gomes Rodrigues <ra...@gmail.com>:

> Hi all,
>
> Anybody have the time to review it and merge it if it's ok?
> If you think I need more work on this PR, I can do it
>
> Thank
> Antonio
>
> 2016-03-22 16:24 GMT+01:00 Antonio Gomes Rodrigues <ra...@gmail.com>:
>
>> Hi all,
>>
>> I would like to add new enhancements to CSV Data Set with new PR
>>
>> For this PR, is it ok or I need rework it to be integrated?
>>
>>
>> Thank
>> Antonio
>>
>> 2016-03-19 9:30 GMT+01:00 Antonio Gomes Rodrigues <ra...@gmail.com>:
>>
>>> Hi all,
>>>
>>> Do I need to modify something to allow the PR be accepted?
>>>
>>> Thank
>>> Antonio
>>>
>>> 2016-03-17 14:45 GMT+01:00 Antonio Gomes Rodrigues <ra...@gmail.com>:
>>>
>>>> Hi,
>>>>
>>>> Unitary tests fixed
>>>>
>>>> Antonio
>>>>
>>>> Cet e-mail a été envoyé depuis un ordinateur protégé par Avast.
>>>> www.avast.com
>>>> <https://www.avast.com/fr-fr/lp-esg-fav?utm_medium=email&utm_source=link&utm_campaign=sig-email&utm_content=webmail&utm_term=OA-2109-B>
>>>> <#m_3957883890638219315_m_7294166784504206657_-5108386039962379889_6858209084203675670_DDB4FAA8-2DD7-40BB-A1B8-4E2AA1F9FDF2>
>>>>
>>>> 2016-03-17 12:38 GMT+01:00 sebb <se...@gmail.com>:
>>>>
>>>>> On 17 March 2016 at 11:20, Antonio Gomes Rodrigues <ra...@gmail.com>
>>>>> wrote:
>>>>> > I will fix the unit test asap
>>>>> >
>>>>> > Can you provide the checkstyle configuration to run it in local
>>>>> before
>>>>> > commit the change?
>>>>>
>>>>> It's in SVN.
>>>>>
>>>>> But as already noted it does not catch everything.
>>>>>
>>>>> The best is to follow the layout convention for the file that you are
>>>>> working on.
>>>>> This may vary between files (and certainly does for source file types).
>>>>>
>>>>> > Antonio
>>>>> >
>>>>> > Cet e-mail a été envoyé depuis un ordinateur protégé par Avast.
>>>>> > www.avast.com
>>>>> > <
>>>>> https://www.avast.com/fr-fr/lp-esg-fav?utm_medium=email&utm_source=link&utm_campaign=sig-email&utm_content=webmail&utm_term=OA-2109-B
>>>>> >
>>>>> > <#DDB4FAA8-2DD7-40BB-A1B8-4E2AA1F9FDF2>
>>>>> >
>>>>> > 2016-03-17 12:15 GMT+01:00 Vladimir Sitnikov <
>>>>> sitnikov.vladimir@gmail.com>:
>>>>> >
>>>>> >> Antonio>Do we have a checkstyle configuration or something like that
>>>>> >> to avoid waste
>>>>> >> Antonio>time (your time and mine) in how to code to JMeter?
>>>>> >>
>>>>> >> There's a checkstyle (it is run as a part of Travis job), however it
>>>>> >> would never check 100% of the issues.
>>>>> >> My "else branch swap" request was just to make sure the diff does
>>>>> not
>>>>> >> bring unintentional modifications.
>>>>> >>
>>>>> >> By the way, have you seen that Travis job fails? Any chances your
>>>>> fix that?
>>>>> >>
>>>>> >> Vladimir
>>>>> >>
>>>>>
>>>>
>>>>
>>>
>>
>

Re: [GitHub] jmeter pull request: bug59153_CSVDataSetFilesExceptions

Posted by Antonio Gomes Rodrigues <ra...@gmail.com>.
Hi all,

Anybody have the time to review it and merge it if it's ok?
If you think I need more work on this PR, I can do it

Thank
Antonio

2016-03-22 16:24 GMT+01:00 Antonio Gomes Rodrigues <ra...@gmail.com>:

> Hi all,
>
> I would like to add new enhancements to CSV Data Set with new PR
>
> For this PR, is it ok or I need rework it to be integrated?
>
>
> Thank
> Antonio
>
> 2016-03-19 9:30 GMT+01:00 Antonio Gomes Rodrigues <ra...@gmail.com>:
>
>> Hi all,
>>
>> Do I need to modify something to allow the PR be accepted?
>>
>> Thank
>> Antonio
>>
>> 2016-03-17 14:45 GMT+01:00 Antonio Gomes Rodrigues <ra...@gmail.com>:
>>
>>> Hi,
>>>
>>> Unitary tests fixed
>>>
>>> Antonio
>>>
>>> Cet e-mail a été envoyé depuis un ordinateur protégé par Avast.
>>> www.avast.com
>>> <https://www.avast.com/fr-fr/lp-esg-fav?utm_medium=email&utm_source=link&utm_campaign=sig-email&utm_content=webmail&utm_term=OA-2109-B>
>>> <#m_7294166784504206657_-5108386039962379889_6858209084203675670_DDB4FAA8-2DD7-40BB-A1B8-4E2AA1F9FDF2>
>>>
>>> 2016-03-17 12:38 GMT+01:00 sebb <se...@gmail.com>:
>>>
>>>> On 17 March 2016 at 11:20, Antonio Gomes Rodrigues <ra...@gmail.com>
>>>> wrote:
>>>> > I will fix the unit test asap
>>>> >
>>>> > Can you provide the checkstyle configuration to run it in local before
>>>> > commit the change?
>>>>
>>>> It's in SVN.
>>>>
>>>> But as already noted it does not catch everything.
>>>>
>>>> The best is to follow the layout convention for the file that you are
>>>> working on.
>>>> This may vary between files (and certainly does for source file types).
>>>>
>>>> > Antonio
>>>> >
>>>> > Cet e-mail a été envoyé depuis un ordinateur protégé par Avast.
>>>> > www.avast.com
>>>> > <
>>>> https://www.avast.com/fr-fr/lp-esg-fav?utm_medium=email&utm_source=link&utm_campaign=sig-email&utm_content=webmail&utm_term=OA-2109-B
>>>> >
>>>> > <#DDB4FAA8-2DD7-40BB-A1B8-4E2AA1F9FDF2>
>>>> >
>>>> > 2016-03-17 12:15 GMT+01:00 Vladimir Sitnikov <
>>>> sitnikov.vladimir@gmail.com>:
>>>> >
>>>> >> Antonio>Do we have a checkstyle configuration or something like that
>>>> >> to avoid waste
>>>> >> Antonio>time (your time and mine) in how to code to JMeter?
>>>> >>
>>>> >> There's a checkstyle (it is run as a part of Travis job), however it
>>>> >> would never check 100% of the issues.
>>>> >> My "else branch swap" request was just to make sure the diff does not
>>>> >> bring unintentional modifications.
>>>> >>
>>>> >> By the way, have you seen that Travis job fails? Any chances your
>>>> fix that?
>>>> >>
>>>> >> Vladimir
>>>> >>
>>>>
>>>
>>>
>>
>

Re: [GitHub] jmeter pull request: bug59153_CSVDataSetFilesExceptions

Posted by Antonio Gomes Rodrigues <ra...@gmail.com>.
Hi all,

I would like to add new enhancements to CSV Data Set with new PR

For this PR, is it ok or I need rework it to be integrated?


Thank
Antonio

2016-03-19 9:30 GMT+01:00 Antonio Gomes Rodrigues <ra...@gmail.com>:

> Hi all,
>
> Do I need to modify something to allow the PR be accepted?
>
> Thank
> Antonio
>
> 2016-03-17 14:45 GMT+01:00 Antonio Gomes Rodrigues <ra...@gmail.com>:
>
>> Hi,
>>
>> Unitary tests fixed
>>
>> Antonio
>>
>> Cet e-mail a été envoyé depuis un ordinateur protégé par Avast.
>> www.avast.com
>> <https://www.avast.com/fr-fr/lp-esg-fav?utm_medium=email&utm_source=link&utm_campaign=sig-email&utm_content=webmail&utm_term=OA-2109-B>
>> <#-5108386039962379889_6858209084203675670_DDB4FAA8-2DD7-40BB-A1B8-4E2AA1F9FDF2>
>>
>> 2016-03-17 12:38 GMT+01:00 sebb <se...@gmail.com>:
>>
>>> On 17 March 2016 at 11:20, Antonio Gomes Rodrigues <ra...@gmail.com>
>>> wrote:
>>> > I will fix the unit test asap
>>> >
>>> > Can you provide the checkstyle configuration to run it in local before
>>> > commit the change?
>>>
>>> It's in SVN.
>>>
>>> But as already noted it does not catch everything.
>>>
>>> The best is to follow the layout convention for the file that you are
>>> working on.
>>> This may vary between files (and certainly does for source file types).
>>>
>>> > Antonio
>>> >
>>> > Cet e-mail a été envoyé depuis un ordinateur protégé par Avast.
>>> > www.avast.com
>>> > <
>>> https://www.avast.com/fr-fr/lp-esg-fav?utm_medium=email&utm_source=link&utm_campaign=sig-email&utm_content=webmail&utm_term=OA-2109-B
>>> >
>>> > <#DDB4FAA8-2DD7-40BB-A1B8-4E2AA1F9FDF2>
>>> >
>>> > 2016-03-17 12:15 GMT+01:00 Vladimir Sitnikov <
>>> sitnikov.vladimir@gmail.com>:
>>> >
>>> >> Antonio>Do we have a checkstyle configuration or something like that
>>> >> to avoid waste
>>> >> Antonio>time (your time and mine) in how to code to JMeter?
>>> >>
>>> >> There's a checkstyle (it is run as a part of Travis job), however it
>>> >> would never check 100% of the issues.
>>> >> My "else branch swap" request was just to make sure the diff does not
>>> >> bring unintentional modifications.
>>> >>
>>> >> By the way, have you seen that Travis job fails? Any chances your fix
>>> that?
>>> >>
>>> >> Vladimir
>>> >>
>>>
>>
>>
>

Re: [GitHub] jmeter pull request: bug59153_CSVDataSetFilesExceptions

Posted by Antonio Gomes Rodrigues <ra...@gmail.com>.
Hi all,

Do I need to modify something to allow the PR be accepted?

Thank
Antonio

2016-03-17 14:45 GMT+01:00 Antonio Gomes Rodrigues <ra...@gmail.com>:

> Hi,
>
> Unitary tests fixed
>
> Antonio
>
> Cet e-mail a été envoyé depuis un ordinateur protégé par Avast.
> www.avast.com
> <https://www.avast.com/fr-fr/lp-esg-fav?utm_medium=email&utm_source=link&utm_campaign=sig-email&utm_content=webmail&utm_term=OA-2109-B>
> <#6858209084203675670_DDB4FAA8-2DD7-40BB-A1B8-4E2AA1F9FDF2>
>
> 2016-03-17 12:38 GMT+01:00 sebb <se...@gmail.com>:
>
>> On 17 March 2016 at 11:20, Antonio Gomes Rodrigues <ra...@gmail.com>
>> wrote:
>> > I will fix the unit test asap
>> >
>> > Can you provide the checkstyle configuration to run it in local before
>> > commit the change?
>>
>> It's in SVN.
>>
>> But as already noted it does not catch everything.
>>
>> The best is to follow the layout convention for the file that you are
>> working on.
>> This may vary between files (and certainly does for source file types).
>>
>> > Antonio
>> >
>> > Cet e-mail a été envoyé depuis un ordinateur protégé par Avast.
>> > www.avast.com
>> > <
>> https://www.avast.com/fr-fr/lp-esg-fav?utm_medium=email&utm_source=link&utm_campaign=sig-email&utm_content=webmail&utm_term=OA-2109-B
>> >
>> > <#DDB4FAA8-2DD7-40BB-A1B8-4E2AA1F9FDF2>
>> >
>> > 2016-03-17 12:15 GMT+01:00 Vladimir Sitnikov <
>> sitnikov.vladimir@gmail.com>:
>> >
>> >> Antonio>Do we have a checkstyle configuration or something like that
>> >> to avoid waste
>> >> Antonio>time (your time and mine) in how to code to JMeter?
>> >>
>> >> There's a checkstyle (it is run as a part of Travis job), however it
>> >> would never check 100% of the issues.
>> >> My "else branch swap" request was just to make sure the diff does not
>> >> bring unintentional modifications.
>> >>
>> >> By the way, have you seen that Travis job fails? Any chances your fix
>> that?
>> >>
>> >> Vladimir
>> >>
>>
>
>

Re: [GitHub] jmeter pull request: bug59153_CSVDataSetFilesExceptions

Posted by Antonio Gomes Rodrigues <ra...@gmail.com>.
Hi,

Unitary tests fixed

Antonio

Cet e-mail a été envoyé depuis un ordinateur protégé par Avast.
www.avast.com
<https://www.avast.com/fr-fr/lp-esg-fav?utm_medium=email&utm_source=link&utm_campaign=sig-email&utm_content=webmail&utm_term=OA-2109-B>
<#DDB4FAA8-2DD7-40BB-A1B8-4E2AA1F9FDF2>

2016-03-17 12:38 GMT+01:00 sebb <se...@gmail.com>:

> On 17 March 2016 at 11:20, Antonio Gomes Rodrigues <ra...@gmail.com>
> wrote:
> > I will fix the unit test asap
> >
> > Can you provide the checkstyle configuration to run it in local before
> > commit the change?
>
> It's in SVN.
>
> But as already noted it does not catch everything.
>
> The best is to follow the layout convention for the file that you are
> working on.
> This may vary between files (and certainly does for source file types).
>
> > Antonio
> >
> > Cet e-mail a été envoyé depuis un ordinateur protégé par Avast.
> > www.avast.com
> > <
> https://www.avast.com/fr-fr/lp-esg-fav?utm_medium=email&utm_source=link&utm_campaign=sig-email&utm_content=webmail&utm_term=OA-2109-B
> >
> > <#DDB4FAA8-2DD7-40BB-A1B8-4E2AA1F9FDF2>
> >
> > 2016-03-17 12:15 GMT+01:00 Vladimir Sitnikov <
> sitnikov.vladimir@gmail.com>:
> >
> >> Antonio>Do we have a checkstyle configuration or something like that
> >> to avoid waste
> >> Antonio>time (your time and mine) in how to code to JMeter?
> >>
> >> There's a checkstyle (it is run as a part of Travis job), however it
> >> would never check 100% of the issues.
> >> My "else branch swap" request was just to make sure the diff does not
> >> bring unintentional modifications.
> >>
> >> By the way, have you seen that Travis job fails? Any chances your fix
> that?
> >>
> >> Vladimir
> >>
>

Re: [GitHub] jmeter pull request: bug59153_CSVDataSetFilesExceptions

Posted by sebb <se...@gmail.com>.
On 17 March 2016 at 11:20, Antonio Gomes Rodrigues <ra...@gmail.com> wrote:
> I will fix the unit test asap
>
> Can you provide the checkstyle configuration to run it in local before
> commit the change?

It's in SVN.

But as already noted it does not catch everything.

The best is to follow the layout convention for the file that you are
working on.
This may vary between files (and certainly does for source file types).

> Antonio
>
> Cet e-mail a été envoyé depuis un ordinateur protégé par Avast.
> www.avast.com
> <https://www.avast.com/fr-fr/lp-esg-fav?utm_medium=email&utm_source=link&utm_campaign=sig-email&utm_content=webmail&utm_term=OA-2109-B>
> <#DDB4FAA8-2DD7-40BB-A1B8-4E2AA1F9FDF2>
>
> 2016-03-17 12:15 GMT+01:00 Vladimir Sitnikov <si...@gmail.com>:
>
>> Antonio>Do we have a checkstyle configuration or something like that
>> to avoid waste
>> Antonio>time (your time and mine) in how to code to JMeter?
>>
>> There's a checkstyle (it is run as a part of Travis job), however it
>> would never check 100% of the issues.
>> My "else branch swap" request was just to make sure the diff does not
>> bring unintentional modifications.
>>
>> By the way, have you seen that Travis job fails? Any chances your fix that?
>>
>> Vladimir
>>

Re: [GitHub] jmeter pull request: bug59153_CSVDataSetFilesExceptions

Posted by Antonio Gomes Rodrigues <ra...@gmail.com>.
I will fix the unit test asap

Can you provide the checkstyle configuration to run it in local before
commit the change?

Antonio

Cet e-mail a été envoyé depuis un ordinateur protégé par Avast.
www.avast.com
<https://www.avast.com/fr-fr/lp-esg-fav?utm_medium=email&utm_source=link&utm_campaign=sig-email&utm_content=webmail&utm_term=OA-2109-B>
<#DDB4FAA8-2DD7-40BB-A1B8-4E2AA1F9FDF2>

2016-03-17 12:15 GMT+01:00 Vladimir Sitnikov <si...@gmail.com>:

> Antonio>Do we have a checkstyle configuration or something like that
> to avoid waste
> Antonio>time (your time and mine) in how to code to JMeter?
>
> There's a checkstyle (it is run as a part of Travis job), however it
> would never check 100% of the issues.
> My "else branch swap" request was just to make sure the diff does not
> bring unintentional modifications.
>
> By the way, have you seen that Travis job fails? Any chances your fix that?
>
> Vladimir
>

Re: [GitHub] jmeter pull request: bug59153_CSVDataSetFilesExceptions

Posted by Vladimir Sitnikov <si...@gmail.com>.
Antonio>Do we have a checkstyle configuration or something like that
to avoid waste
Antonio>time (your time and mine) in how to code to JMeter?

There's a checkstyle (it is run as a part of Travis job), however it
would never check 100% of the issues.
My "else branch swap" request was just to make sure the diff does not
bring unintentional modifications.

By the way, have you seen that Travis job fails? Any chances your fix that?

Vladimir

Re: [GitHub] jmeter pull request: bug59153_CSVDataSetFilesExceptions

Posted by Antonio Gomes Rodrigues <ra...@gmail.com>.
Hi,


Modification made and new commit provided

Do we have a checkstyle configuration or something like that to avoid waste
time (your time and mine) in how to code to JMeter?

Antonio

Cet e-mail a été envoyé depuis un ordinateur protégé par Avast.
www.avast.com
<https://www.avast.com/fr-fr/lp-esg-fav?utm_medium=email&utm_source=link&utm_campaign=sig-email&utm_content=webmail&utm_term=OA-2109-B>
<#DDB4FAA8-2DD7-40BB-A1B8-4E2AA1F9FDF2>

2016-03-17 12:02 GMT+01:00 vlsi <gi...@git.apache.org>:

> Github user vlsi commented on a diff in the pull request:
>
>     https://github.com/apache/jmeter/pull/167#discussion_r56486961
>
>     --- Diff: src/core/org/apache/jmeter/services/FileServer.java ---
>     @@ -428,7 +431,7 @@ private BufferedReader
> createBufferedReader(FileEntry fileEntry) throws IOExcept
>                  isr = new InputStreamReader(fis);
>              }
>              return new BufferedReader(isr);
>     -    }
>     +        }
>     --- End diff --
>
>     Would you avoid adding extra spaces?
>
>
> ---
> If your project is set up for it, you can reply to this email and have your
> reply appear on GitHub as well. If your project does not have this feature
> enabled and wishes so, or if the feature is enabled but not working, please
> contact infrastructure at infrastructure@apache.org or file a JIRA ticket
> with INFRA.
> ---
>

[GitHub] jmeter pull request: bug59153_CSVDataSetFilesExceptions

Posted by vlsi <gi...@git.apache.org>.
Github user vlsi commented on a diff in the pull request:

    https://github.com/apache/jmeter/pull/167#discussion_r56486961
  
    --- Diff: src/core/org/apache/jmeter/services/FileServer.java ---
    @@ -428,7 +431,7 @@ private BufferedReader createBufferedReader(FileEntry fileEntry) throws IOExcept
                 isr = new InputStreamReader(fis);
             }
             return new BufferedReader(isr);
    -    }
    +        }
    --- End diff --
    
    Would you avoid adding extra spaces?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] jmeter pull request: bug59153_CSVDataSetFilesExceptions

Posted by ra0077 <gi...@git.apache.org>.
Github user ra0077 commented on the pull request:

    https://github.com/apache/jmeter/pull/167#issuecomment-197735103
  
    New commit with formating and swap else branch


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---