You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "jianghuazhu (via GitHub)" <gi...@apache.org> on 2023/11/05 05:51:42 UTC
[PR] HDDS-9630.Improve some documents related to ECReplicationConfig [ozone]
jianghuazhu opened a new pull request, #5550:
URL: https://github.com/apache/ozone/pull/5550
## What changes were proposed in this pull request?
There is some missing documentation and comments in ECReplicationConfig, the purpose of this PR is to improve here.
Details: HDDS-9630
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-9630
## How was this patch tested?
This PR is mainly related to documentation and does not require too much testing.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-9630. Improve some documents related to ECReplicationConfig [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on code in PR #5550:
URL: https://github.com/apache/ozone/pull/5550#discussion_r1382926349
##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/client/ECReplicationConfig.java:
##########
@@ -82,14 +82,17 @@ public ECReplicationConfig(int data, int parity, EcCodec codec,
}
/**
- * Create an ECReplicationConfig object from a string representing the
- * various parameters. Acceptable patterns are like:
+ * Create an {@link ECReplicationConfig} object from a string representing
Review Comment:
I don't think linking to `ECReplicationConfig` from the javadoc of `ECReplicationConfig` (i.e. the same class) adds any value.
##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/client/ECReplicationConfig.java:
##########
@@ -82,14 +82,17 @@ public ECReplicationConfig(int data, int parity, EcCodec codec,
}
/**
- * Create an ECReplicationConfig object from a string representing the
- * various parameters. Acceptable patterns are like:
+ * Create an {@link ECReplicationConfig} object from a string representing
+ * the various parameters. Acceptable patterns are like:
* rs-3-2-1024k
* RS-3-2-2048k
* XOR-10-4-4096K
* IllegalArgumentException will be thrown if the passed string does not
* match the defined pattern.
- * @param string
+ * @param string This is a string parameter used to create an
+ * {@link ECReplicationConfig} instance. It must meet the
+ * requirements, otherwise {@link IllegalArgumentException} will be
+ * thrown.
Review Comment:
I think the same is already described in the existing javadoc. No need to repeat it in different words.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-9630. Improve some documents related to ECReplicationConfig [ozone]
Posted by "jianghuazhu (via GitHub)" <gi...@apache.org>.
jianghuazhu commented on PR #5550:
URL: https://github.com/apache/ozone/pull/5550#issuecomment-1793999585
Thanks for your help, @dombizita .
I noticed that for first time contributors, maintainer approval is required here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-9630. Add missing javadoc description for ECReplicationConfig constructor param [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai merged PR #5550:
URL: https://github.com/apache/ozone/pull/5550
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-9630. Improve some documents related to ECReplicationConfig [ozone]
Posted by "dombizita (via GitHub)" <gi...@apache.org>.
dombizita commented on PR #5550:
URL: https://github.com/apache/ozone/pull/5550#issuecomment-1793859137
Hi @jianghuazhu, thanks for working on this! Please enable the build-branch workflow in [your fork](https://github.com/jianghuazhu/ozone/actions), at first you can run the CI there and check if there are any failures.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-9630. Improve some documents related to ECReplicationConfig [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on code in PR #5550:
URL: https://github.com/apache/ozone/pull/5550#discussion_r1382928721
##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/client/ECReplicationConfig.java:
##########
@@ -82,14 +82,17 @@ public ECReplicationConfig(int data, int parity, EcCodec codec,
}
/**
- * Create an ECReplicationConfig object from a string representing the
- * various parameters. Acceptable patterns are like:
+ * Create an {@link ECReplicationConfig} object from a string representing
+ * the various parameters. Acceptable patterns are like:
* rs-3-2-1024k
* RS-3-2-2048k
* XOR-10-4-4096K
* IllegalArgumentException will be thrown if the passed string does not
* match the defined pattern.
- * @param string
+ * @param string This is a string parameter used to create an
+ * {@link ECReplicationConfig} instance. It must meet the
+ * requirements, otherwise {@link IllegalArgumentException} will be
+ * thrown.
Review Comment:
I think the same is already described in the existing javadoc. No need to repeat it in different words. Adding a description like "EC replication config in the format <codec>-<data>-<parity>-<chunksize>" would be enough.
##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/client/ECReplicationConfig.java:
##########
@@ -82,14 +82,17 @@ public ECReplicationConfig(int data, int parity, EcCodec codec,
}
/**
- * Create an ECReplicationConfig object from a string representing the
- * various parameters. Acceptable patterns are like:
+ * Create an {@link ECReplicationConfig} object from a string representing
+ * the various parameters. Acceptable patterns are like:
* rs-3-2-1024k
* RS-3-2-2048k
* XOR-10-4-4096K
* IllegalArgumentException will be thrown if the passed string does not
* match the defined pattern.
- * @param string
+ * @param string This is a string parameter used to create an
+ * {@link ECReplicationConfig} instance. It must meet the
+ * requirements, otherwise {@link IllegalArgumentException} will be
+ * thrown.
Review Comment:
I think the same is already described in the existing javadoc. No need to repeat it in different words. Adding a description like `EC replication config in the format <codec>-<data>-<parity>-<chunksize>` would be enough.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-9630.Improve some documentation when creating ECReplicationConfig instances [ozone]
Posted by "jianghuazhu (via GitHub)" <gi...@apache.org>.
jianghuazhu commented on code in PR #5550:
URL: https://github.com/apache/ozone/pull/5550#discussion_r1383269676
##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/client/ECReplicationConfig.java:
##########
@@ -89,7 +89,8 @@ public ECReplicationConfig(int data, int parity, EcCodec codec,
* XOR-10-4-4096K
* IllegalArgumentException will be thrown if the passed string does not
* match the defined pattern.
- * @param string
+ * @param string Parameters used to create ECReplicationConfig instances.
+ * Format: <codec>-<data>-<parity>-<chunksize>
Review Comment:
Thanks @adoroszlai , I'll update soon.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-9630.Improve some documentation when creating ECReplicationConfig instances [ozone]
Posted by "jianghuazhu (via GitHub)" <gi...@apache.org>.
jianghuazhu commented on PR #5550:
URL: https://github.com/apache/ozone/pull/5550#issuecomment-1794848602
I made some updates, can you help review the PR again, @adoroszlai .
Thanks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-9630. Improve some documents related to ECReplicationConfig [ozone]
Posted by "jianghuazhu (via GitHub)" <gi...@apache.org>.
jianghuazhu commented on code in PR #5550:
URL: https://github.com/apache/ozone/pull/5550#discussion_r1382945166
##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/client/ECReplicationConfig.java:
##########
@@ -82,14 +82,17 @@ public ECReplicationConfig(int data, int parity, EcCodec codec,
}
/**
- * Create an ECReplicationConfig object from a string representing the
- * various parameters. Acceptable patterns are like:
+ * Create an {@link ECReplicationConfig} object from a string representing
+ * the various parameters. Acceptable patterns are like:
* rs-3-2-1024k
* RS-3-2-2048k
* XOR-10-4-4096K
* IllegalArgumentException will be thrown if the passed string does not
* match the defined pattern.
- * @param string
+ * @param string This is a string parameter used to create an
+ * {@link ECReplicationConfig} instance. It must meet the
+ * requirements, otherwise {@link IllegalArgumentException} will be
+ * thrown.
Review Comment:
Thanks @adoroszlai for the comment and review.
I will update later.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-9630.Improve some documentation when creating ECReplicationConfig instances [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on code in PR #5550:
URL: https://github.com/apache/ozone/pull/5550#discussion_r1383226661
##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/client/ECReplicationConfig.java:
##########
@@ -89,7 +89,8 @@ public ECReplicationConfig(int data, int parity, EcCodec codec,
* XOR-10-4-4096K
* IllegalArgumentException will be thrown if the passed string does not
* match the defined pattern.
- * @param string
+ * @param string Parameters used to create ECReplicationConfig instances.
+ * Format: <codec>-<data>-<parity>-<chunksize>
Review Comment:
Javadoc allows HTML tags, so `<` and `>` should be written as `<` and `>`, otherwise this will be rendered as `----`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-9630.Improve some documents related to ECReplicationConfig [ozone]
Posted by "jianghuazhu (via GitHub)" <gi...@apache.org>.
jianghuazhu commented on PR #5550:
URL: https://github.com/apache/ozone/pull/5550#issuecomment-1793666101
Hi, @adoroszlai @sodonnel , I am a newbie and CI/CD execution cannot be triggered here, can you guys help me?
If possible, please help review this PR. Thanks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-9630. Improve some documents related to ECReplicationConfig [ozone]
Posted by "jianghuazhu (via GitHub)" <gi...@apache.org>.
jianghuazhu commented on PR #5550:
URL: https://github.com/apache/ozone/pull/5550#issuecomment-1794104714
Thanks for your help, @guohao-rosicky .
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-9630.Improve some documentation when creating ECReplicationConfig instances [ozone]
Posted by "jianghuazhu (via GitHub)" <gi...@apache.org>.
jianghuazhu commented on PR #5550:
URL: https://github.com/apache/ozone/pull/5550#issuecomment-1794618254
I made some updates, can you help review the PR again, @adoroszlai .
Thanks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-9630. Add missing javadoc description for ECReplicationConfig constructor param [ozone]
Posted by "jianghuazhu (via GitHub)" <gi...@apache.org>.
jianghuazhu commented on PR #5550:
URL: https://github.com/apache/ozone/pull/5550#issuecomment-1800883220
Among the results of CI/CD, there was a failure. But it seems to have little to do with this PR. @adoroszlai
https://github.com/apache/ozone/actions/runs/6771440897/job/18460860550?pr=5550
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-9630. Improve some documents related to ECReplicationConfig [ozone]
Posted by "guohao-rosicky (via GitHub)" <gi...@apache.org>.
guohao-rosicky commented on PR #5550:
URL: https://github.com/apache/ozone/pull/5550#issuecomment-1794032031
Thanks @jianghuazhu work on this, let's waiting ci passed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org