Re: [GitHub] jmeter pull request: Use explicit timeout for TestDNSCacheManager,...
>So does that mean that PRs are mutable? Of course they are. Note: 1) If you access PR via git interface, you have immediate visibility that PR is updated. There's no way to mutate the contents of a commit. Its Id would have to be updated. 2) In github.com UI, commits/comments are ordered by date, so it is obvious when commit was created afterwards. Just go ahead and check that my commit is below Philippe's comment: https://github.com/apache/jmeter/pull/176#issuecomment-204965711 Note: each commit is immutable, however one can update the PR with _another_ set of commits. That is useful for: 1) Corrections for code review 2) Merging multiple commits into a single one. For instance, Philippe's https://github.com/apache/jmeter/pull/179 consists of 7 commits, however I do not see much value in publishing 7 "work-in-progress" commits. Well, it might make sense if you publish one, then another (so other can see the diff), but in general single commit per feature makes more sense for me. Vladimir
Re: [GitHub] jmeter pull request: Use explicit timeout for TestDNSCacheManager,...
On 3 April 2016 at 14:38, vlsi wrote: > Github user vlsi commented on the pull request: > > https://github.com/apache/jmeter/pull/176#issuecomment-204978579 > > > Sorry , I had some hallucination although I checked twice :-) , it is > package protected. > > You are fine. I've just updated the PR as you suggested package-private. > So does that mean that PRs are mutable? That seems like a recipe for confusion (as we have just seen) and potentially worse. > --- > 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 infrastruct...@apache.org or file a JIRA ticket > with INFRA. > ---
[GitHub] jmeter pull request: Use explicit timeout for TestDNSCacheManager,...
Github user asfgit closed the pull request at: https://github.com/apache/jmeter/pull/176 --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] jmeter pull request: Use explicit timeout for TestDNSCacheManager,...
Github user vlsi commented on the pull request: https://github.com/apache/jmeter/pull/176#issuecomment-204978579 > Sorry , I had some hallucination although I checked twice :-) , it is package protected. You are fine. I've just updated the PR as you suggested package-private. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] jmeter pull request: Use explicit timeout for TestDNSCacheManager,...
Github user pmouawad commented on the pull request: https://github.com/apache/jmeter/pull/176#issuecomment-204978538 Sorry , I had some hallucination although I checked twice :-) , it is package protected. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] jmeter pull request: Use explicit timeout for TestDNSCacheManager,...
Github user pmouawad commented on the pull request: https://github.com/apache/jmeter/pull/176#issuecomment-204978544 +1 for me --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
Re: [GitHub] jmeter pull request: Use explicit timeout for TestDNSCacheManager,...
On 3 April 2016 at 13:48, pmouawad wrote: > Github user pmouawad commented on the pull request: > > https://github.com/apache/jmeter/pull/176#issuecomment-204965711 > > Hi Vladimir, > Exposing public method for testing might introduce issues. > Users can think that field is persisted in JMX while it's not. > > Can't it be package protected ? > Huh? Looks to me as though it is package protected. But it would be good to add a comment to say it is only for unit tests. Not sure the Javadoc is needed for package methods; it may suggest these methods is supported, so I would replace it with something like: // for unit testing purposes only > --- > 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 infrastruct...@apache.org or file a JIRA ticket > with INFRA. > ---
[GitHub] jmeter pull request: Use explicit timeout for TestDNSCacheManager,...
Github user pmouawad commented on the pull request: https://github.com/apache/jmeter/pull/176#issuecomment-204965711 Hi Vladimir, Exposing public method for testing might introduce issues. Users can think that field is persisted in JMX while it's not. Can't it be package protected ? --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] jmeter pull request: Use explicit timeout for TestDNSCacheManager,...
GitHub user vlsi opened a pull request: https://github.com/apache/jmeter/pull/176 Use explicit timeout for TestDNSCacheManager, so test is executed faster `org.apache.jmeter.protocol.http.control.TestDNSCacheManager#testCloneWithCustomResolverAndInvalidNameserver` took 30 seconds to execute. You can merge this pull request into a Git repository by running: $ git pull https://github.com/vlsi/jmeter dns_timeout Alternatively you can review and apply these changes as the patch at: https://github.com/apache/jmeter/pull/176.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 #176 commit a2e17f8061a8916fb751f6d5a908165bd424d6b1 Author: Vladimir Sitnikov Date: 2016-04-03T00:15:56Z Use explicit timeout for TestDNSCacheManager, so test is executed faster --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
