New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PAYARA-3061 Cleanup nucleus admin server mgmt #3115

Merged
merged 7 commits into from Sep 19, 2018

Conversation

Projects
None yet
4 participants
@Cousjava
Member

Cousjava commented Sep 3, 2018

No description provided.

@Cousjava Cousjava added this to the Payara 5.184 milestone Sep 3, 2018

@Cousjava

This comment has been minimized.

Member

Cousjava commented Sep 3, 2018

Jenkins test please

@payara-ci

This comment has been minimized.

Contributor

payara-ci commented Sep 3, 2018

Quick build and test passed!

@Cousjava Cousjava changed the title from Cleanup nucleus admin server mgmt to PAYARA-3061 Cleanup nucleus admin server mgmt Sep 3, 2018

@MarkWareham

This comment has been minimized.

Contributor

MarkWareham commented Sep 4, 2018

Any chance you could break this down into smaller PRs? It would be easier to review and revert if needed

@Pandrex247

TLDR: 1) Move monitoring cleanup into its own PR 2) Don't create empty javadoc 3) entry not entrie 4) careful of scope changes

@@ -107,7 +107,7 @@
/**
*
* Starts AMX monitoring

This comment has been minimized.

@Pandrex247

Pandrex247 Sep 11, 2018

Member

Please split out the changes of the monitoring module into a separate PR.

@ManagedAttribute(id="currentthreadcputime")
@Description( "Returns the total CPU time for the current thread in nanoseconds" )
public CountStatistic getCurrentThreadCpuTime() {
this.currentThreadCpuTime.setCount(threadBean.getCurrentThreadCpuTime());
return this.currentThreadCpuTime;
}
/**
*

This comment has been minimized.

@Pandrex247

Pandrex247 Sep 11, 2018

Member

Empty Javadoc

@@ -72,12 +72,12 @@
private long startTime = 0;
private AtomicLong lastSampleTime = new AtomicLong(0);
private static final String NAME = "average";
private static final String DESCRIPTION = "Average RangeStatistic";
private static final String AVERAGE_NAME = "average";

This comment has been minimized.

@Pandrex247

Pandrex247 Sep 11, 2018

Member

Was fine as is.

This comment has been minimized.

@Cousjava

Cousjava Sep 11, 2018

Member

It is a blocker code smell on sonarcloud as there is also the field description (non-capitalised)

@@ -66,7 +66,7 @@
long max = 0;
long min = 0;
private final AtomicLong lastSampleTime = new AtomicLong();
private static final String DESCRIPTION = "Counter CountStatistic";
private static final String COUNTER_DESCRIPTION = "Counter CountStatistic";

This comment has been minimized.

@Pandrex247

Pandrex247 Sep 11, 2018

Member

Was fine as is.

This comment has been minimized.

@Cousjava

Cousjava Sep 11, 2018

Member

It is a blocker code smell on sonarcloud as there is also the field description (non-capitalised)

@@ -118,6 +123,24 @@ public DomainConfig(String domainName, String domainRoot) throws DomainException
/**
* This constructor is used at domain creation time only.
* @param domainName

This comment has been minimized.

@Pandrex247

Pandrex247 Sep 11, 2018

Member

Please don't just make empty javadoc - give them descriptions please.

Show resolved Hide resolved ...src/main/java/com/sun/enterprise/admin/servermgmt/RepositoryManager.java
@@ -142,7 +151,7 @@ protected final boolean isRemote() {
return !isLocal();
}
private final void resetLocalPassword() throws IOException {
private void resetLocalPassword() throws IOException {

This comment has been minimized.

@Pandrex247

Pandrex247 Sep 11, 2018

Member

Is this necessary? There's no other changes referencing this in the PR.

if (up_ms <= 0) {
throw new CommandException(strings.get("restart.dasNotRunning"));
if (upTimeMillis <= 0) {

This comment has been minimized.

@Pandrex247

Pandrex247 Sep 11, 2018

Member

Uptime is one word - the capitalised T isn't necessary.

Show resolved Hide resolved ...n/java/com/sun/enterprise/admin/servermgmt/domain/CustomTokenClient.java
@@ -347,11 +318,49 @@ private void createDirectory(File dir) throws RepositoryException {
if (!dir.exists()) {
try {
if (!dir.mkdirs()) {
throw new RepositoryException(_strings.get("directoryCreationError", dir));
throw new RepositoryException(STRINGS.get("directoryCreationError", dir));

This comment has been minimized.

@Pandrex247

Pandrex247 Sep 11, 2018

Member

Random tab not fixed.

@Cousjava Cousjava force-pushed the Cousjava:cleanup-nucleus-admin-server-mgmt branch from 5f2da00 to f25a9c9 Sep 11, 2018

@Cousjava

This comment has been minimized.

Member

Cousjava commented Sep 11, 2018

Jenkins test please

@payara-ci

This comment has been minimized.

Contributor

payara-ci commented Sep 11, 2018

Quick build and test failed!

@Cousjava Cousjava force-pushed the Cousjava:cleanup-nucleus-admin-server-mgmt branch from b64fd69 to 42d6262 Sep 11, 2018

@Cousjava

This comment has been minimized.

Member

Cousjava commented Sep 11, 2018

Jenkins test please

@payara-ci

This comment has been minimized.

Contributor

payara-ci commented Sep 11, 2018

Quick build and test failed!

@Cousjava Cousjava force-pushed the Cousjava:cleanup-nucleus-admin-server-mgmt branch from 42d6262 to d57f5ae Sep 12, 2018

{
if (entries[i].key.equals(key)) { return entries[i]; }
for (DomainConfigEntryInfo entrie : entries) {
if (entrie.key.equals(key)) {

This comment has been minimized.

@Pandrex247

Pandrex247 Sep 18, 2018

Member

Still not how you spell entry

@Cousjava

This comment has been minimized.

Member

Cousjava commented Sep 18, 2018

Jenkins test please

@payara-ci

This comment has been minimized.

Contributor

payara-ci commented Sep 18, 2018

Quick build and test passed!

@Cousjava Cousjava merged commit 074430a into payara:master Sep 19, 2018

3 checks passed

Payara Quick Build Payara quick build passed!
Details
Payara Quick Build and Test Quick build and test passed!
Details
Payara Quick Test Payara quick tests passed!
Details

Cousjava added a commit to Cousjava/Payara that referenced this pull request Sep 19, 2018

Merge pull request payara#3115 from Cousjava/cleanup-nucleus-admin-se…
…rver-mgmt

PAYARA-3061 Cleanup nucleus admin server-mgmt
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment