#752: Add getKeys to JSch which makes access to all the config values…#753
Conversation
…values possible for troubleshooting.
There was a problem hiding this comment.
I think a new method like this would be more valuable, since it would provide users the entire config and not just the config keys:
public static Map<String, String> getConfig() {
Map<String, String> ret = new HashMap<>();
synchronized (config) {
for (Map.Entry<String, String> entry : config.entrySet()) {
String key = entry.getKey();
if (key.equals("PubkeyAcceptedKeyTypes")) {
key = "PubkeyAcceptedAlgorithms";
}
ret.put(key, entry.getValue());
}
}
return Collections.unmodifiableMap(ret);
}…fig values possible for troubleshooting.
|
Thanks I have updated the PR |
| } | ||
|
|
||
| @Test | ||
| void getConfigKeys() throws Exception { |
There was a problem hiding this comment.
Can we rename this test method, since it is now testing a method named getConfig() and not getConfigKeys()?
…fig values possible for troubleshooting.
| import java.util.HashMap; | ||
| import java.util.Hashtable; | ||
| import java.util.Map; | ||
| import java.util.Set; |
There was a problem hiding this comment.
Can you remove this unused java.util.Set import that SonarQube is pointing out?
| } | ||
|
|
||
| @Test | ||
| void getConfig() throws Exception { |
There was a problem hiding this comment.
SonarQube points out that no method in this test case throws a checked Exception, so can we drop the throws Exception clause?
There was a problem hiding this comment.
all the other tests does the same
There was a problem hiding this comment.
Yes, I'll put on my TODO list to clean that up.
…fig values possible for troubleshooting.
|
norrisjeremy
left a comment
norrisjeremy
left a comment
There was a problem hiding this comment.
Can you squash all the changesets together on this branch into one atomic changeset?
|
@norrisjeremy I will squash it when merging |
|
I think you can make GH have a button "squash and merge" that is what we do at Apache Camel |
|
created a new PR this can be closed |
Ok, that works too. Your choice whether to squash and merge this one or use the new #754. |
|
i suggest you try the "squash and merge" button |
3 participants
… possible for troubleshooting.