-
Notifications
You must be signed in to change notification settings - Fork 11
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
#32 - split api docs #33
Conversation
WalkthroughThe changes in this pull request include modifications to the project documentation and the API. The README file has been reorganized, with sections reordered and renamed for clarity along with minor formatting updates. In the API documentation, a new service called Changes
Sequence Diagram(s)sequenceDiagram
participant C as Client
participant CAS as CurrencyAverageRatesService
participant DS as Data Store
C->>CAS: Request (e.g., fromMonth)
CAS->>DS: Retrieve month rates
DS-->>CAS: Return rates
CAS-->>C: Deliver aggregated rates
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (8)
README.md (1)
16-18
: Add Composer Installation InstructionThe dedicated "Installing via composer" section with its command block is a welcome improvement to the documentation. Consider specifying a language identifier (for example, using “
bash” instead of just “
”) to enhance syntax highlighting and better adhere to markdown lint guidelines.api.md (7)
1-4
: New CurrencyAverageRatesService DocumentationThe introduction for CurrencyAverageRatesService clearly establishes its purpose. As a minor suggestion, consider adding a definite article (e.g., "accessing the average rates") to improve grammatical clarity.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~3-~3: Possible missing article found.
Context: ...rageRatesService This service provides API for accessing average rates published i...(AI_HYDRA_LEO_MISSING_AN)
34-37
: Document the fromDay MethodThe description of the
fromDay
method is concise. Consider elaborating slightly on the structure of the returned dictionary to help users understand its contents better.
52-58
: Document the fromDayBefore MethodThe explanation clearly ties the method’s functionality to its practical use case in bookkeeping. To improve formality, you might rephrase “Which is exactly what this method exposes” to something like “This method returns the rate corresponding to the previous business day.”
🧰 Tools
🪛 LanguageTool
[style] ~57-~57: Consider an alternative for the overused word “exactly”.
Context: ...fore the actual transfer date. Which is exactly what this method exposes.<s...(EXACTLY_PRECISELY)
83-90
: Clarify getMonthTablesA Method DescriptionThe description for the
getMonthTablesA
method is informative. For enhanced readability, consider rephrasing the sentence to “To get the rates, a second iteration is required.”🧰 Tools
🪛 LanguageTool
[typographical] ~86-~86: It seems that a comma is missing.
Context: ...a structure provided by NBP. To get the rates there needs to be second iteration: <d...(IN_ORDER_TO_VB_COMMA)
146-174
: Document getMonthTablesB MethodThe explanation and accompanying example for the
getMonthTablesB
method are detailed and account for error handling with exceptions. A minor punctuation improvement is suggested: inserting a comma within the warning statement will aid readability.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
171-171: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
181-190
: Adjust Warning Heading LevelThe "Warning about missing currencies in table 'B'" is currently formatted as an h5 heading. For a more consistent document structure, consider lowering this to an h4 heading so that the heading levels increment by one.
🧰 Tools
🪛 LanguageTool
[typographical] ~188-~188: It appears that a comma is missing.
Context: ...nt in table from the next day. In such case you should not use thegetRate($rate)
...(DURING_THAT_TIME_COMMA)
🪛 markdownlint-cli2 (0.17.2)
181-181: Heading levels should only increment by one level at a time
Expected: h4; Actual: h5(MD001, heading-increment)
1-356
: General Documentation QualityOverall, the API documentation is well-organized and comprehensive. It features clear examples and concise method descriptions. As a general note, please review fenced code blocks and heading levels to ensure they fully comply with markdown lint guidelines.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~3-~3: Possible missing article found.
Context: ...rageRatesService This service provides API for accessing average rates published i...(AI_HYDRA_LEO_MISSING_AN)
[style] ~57-~57: Consider an alternative for the overused word “exactly”.
Context: ...fore the actual transfer date. Which is exactly what this method exposes.<s...(EXACTLY_PRECISELY)
[typographical] ~86-~86: It seems that a comma is missing.
Context: ...a structure provided by NBP. To get the rates there needs to be second iteration: <d...(IN_ORDER_TO_VB_COMMA)
[typographical] ~188-~188: It appears that a comma is missing.
Context: ...nt in table from the next day. In such case you should not use thegetRate($rate)
...(DURING_THAT_TIME_COMMA)
🪛 markdownlint-cli2 (0.17.2)
25-25: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
77-77: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
107-107: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
137-137: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
171-171: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
181-181: Heading levels should only increment by one level at a time
Expected: h4; Actual: h5(MD001, heading-increment)
217-217: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
247-247: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
273-273: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
302-302: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
328-328: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
352-352: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
README.md
(6 hunks)api.md
(1 hunks)
🧰 Additional context used
🪛 LanguageTool
api.md
[uncategorized] ~3-~3: Possible missing article found.
Context: ...rageRatesService This service provides API for accessing average rates published i...
(AI_HYDRA_LEO_MISSING_AN)
[style] ~57-~57: Consider an alternative for the overused word “exactly”.
Context: ...fore the actual transfer date. Which is exactly what this method exposes.
(EXACTLY_PRECISELY)
[typographical] ~86-~86: It seems that a comma is missing.
Context: ...a structure provided by NBP. To get the rates there needs to be second iteration: <d...
(IN_ORDER_TO_VB_COMMA)
[typographical] ~188-~188: It appears that a comma is missing.
Context: ...nt in table from the next day. In such case you should not use the getRate($rate)
...
(DURING_THAT_TIME_COMMA)
🪛 markdownlint-cli2 (0.17.2)
api.md
25-25: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
77-77: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
107-107: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
137-137: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
171-171: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
181-181: Heading levels should only increment by one level at a time
Expected: h4; Actual: h5
(MD001, heading-increment)
217-217: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
247-247: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
273-273: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
302-302: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
328-328: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
352-352: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
🔇 Additional comments (18)
README.md (5)
9-14
: Enhance Badge Display with Line BreaksThe introduction of multiple badge images with
<br />
tags improves visual separation and overall clarity. Please double-check that these new badges match the project’s style guidelines.
45-45
: Clarify Examples SectionDirecting users to the examples directory is concise and clear. This update helps users quickly locate runnable examples.
49-50
: Introduce Cache Usage SectionThe new "Using cache" section is a valuable addition that explains the caching prerequisites (e.g., the need for a PSR-6 compatible library). Ensure that the details provided are in sync with the underlying caching implementation.
136-138
: Revise Architecture Section HeadingRenaming the "Layers" section to "Architecture" sharpens the focus on the code’s structured design. The updated description clearly emphasizes the separation of concerns.
202-202
: Update Coverage LinkThe updated
[link-coverage]
now properly points to the new GitHub Actions workflow for coverage reporting. Please verify that the URL is correct and accessible.api.md (13)
5-8
: Document the fromMonth MethodThe description for the
fromMonth
method is succinct and sets the context well. The information provided is clear and aligns with the intended usage.
9-32
: Clear Example for fromMonth MethodThe example code effectively demonstrates how to iterate over and print the returned rates. Ensure that the output sample matches the real implementation results.
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
25-25: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
38-51
: Comprehensive fromDay Method ExampleThe example demonstrates the typical usage pattern for extracting a currency rate from a specific day. Everything looks in order.
59-81
: fromDayBefore Method Example is ClearThe provided example sufficiently demonstrates how the
fromDayBefore
method can be used, including the extraction and formatting of returned rate details.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
77-77: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
91-117
: Effective Example for getMonthTablesAThe nested iteration example is well-constructed and clearly demonstrates how to traverse the returned tables and access individual rates.
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
107-107: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
120-143
: Additional getMonthTablesA Usage ExampleThis supplementary example for fetching a specific currency rate offers clear and practical guidance to end users.
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
137-137: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
175-179
: getMonthTablesB Example CompletenessThe closing example clearly illustrates the method’s usage and expected output.
191-223
: Document Currency Trading Rates ServiceThe section covering the Currency Trading Rates Service and its
fromMonth
method is thorough and well-explained. The example code clearly details how to traverse the trading rates.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
217-217: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
227-251
: Document fromEffectiveDay MethodThe description and example for the
fromEffectiveDay
method are clear and effective, enabling users to quickly grasp its purpose.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
247-247: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
253-277
: Document fromTradingDay MethodThe example provided for the
fromTradingDay
method neatly demonstrates its usage and expected output. No issues noted.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
273-273: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
279-308
: Documentation for Gold Rates Service – fromMonthThe
fromMonth
method for the Gold Rates Service is well-documented with a detailed example. Everything appears to be in order.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
302-302: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
311-332
: Document Gold Rates Service fromDay MethodThe example for retrieving a gold rate using the
fromDay
method clearly communicates its functionality.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
328-328: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
334-356
: Document Gold Rates Service fromDayBefore MethodThe provided code example for the
fromDayBefore
method is clear and practical. Ensure the sample output is consistent with the real data format.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
352-352: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (11)
api.md (11)
1-3
: Insert missing article in the service description.
In line 3, consider changing “provides API” to “provides an API” for better grammar and clarity.
Proposed change:-This service provides API for accessing the average rates published in NBP tables. +This service provides an API for accessing the average rates published in NBP tables.🧰 Tools
🪛 LanguageTool
[uncategorized] ~3-~3: Possible missing article found.
Context: ...rageRatesService This service provides API for accessing the average rates publish...(AI_HYDRA_LEO_MISSING_AN)
7-7
: Clarify the description for thefromMonth
method.
Update the sentence to include an article and preposition for clarity.
Proposed change:-Returns flat collection of rates from all NBP tables at given month +Returns a flat collection of rates from all NBP tables for a given month
36-36
: Improve wording in thefromDay
method description.
Consider rephrasing “Returns a dictionary with NBP tables from given day” to “Returns a dictionary of NBP tables for a given day” to enhance clarity.
Proposed change:-Returns a dictionary with NBP tables from given day. +Returns a dictionary of NBP tables for a given day.
55-58
: Fix article usage in explaining the pricing logic.
In lines 55–58, insert the missing article before “currency rate” for grammatical correctness.
Proposed change:-... using currency rate applied in the business day before the actual transfer date. +... using a currency rate applied in the business day before the actual transfer date.🧰 Tools
🪛 LanguageTool
[uncategorized] ~57-~57: Possible missing article found.
Context: ...s for the prices to be calculated using currency rate applied in the business day before...(AI_HYDRA_LEO_MISSING_THE)
189-190
: Punctuate the advisory note about missing currencies in table “B”.
Insert a comma after “In such case” to improve readability. Optionally, consider rewording to “In such cases, you should not use…”.
Proposed change:-In such case you should not use the `getRate($rate)` method but rather +In such case, you should not use the `getRate($rate)` method but rather🧰 Tools
🪛 LanguageTool
[typographical] ~189-~189: It appears that a comma is missing.
Context: ...nt in table from the next day. In such case you should not use thegetRate($rate)
...(DURING_THAT_TIME_COMMA)
198-198
: Refine the description for the trading rates month method.
Insert the definite article to read “from the entire month” for consistency.
Proposed change:-Returns trading rates from entire month. +Returns trading rates from the entire month.
228-230
: Correct the verb form in thefromEffectiveDay
method description.
Changing “Return rates from effective date.” to “Returns rates from the effective date.” improves consistency with other method descriptions.
Proposed change:-Return rates from effective date. +Returns rates from the effective date.
255-255
: Fix verb form for thefromTradingDay
method description.
Replace “Return rates from trading date.” with “Returns rates from the trading date.” for consistency.
Proposed change:-Return rates from trading date. +Returns rates from the trading date.
286-286
: Enhance clarity in the gold rates monthly method description.
Consider updating “Gets all rates from specific month.” to “Gets all rates for a specific month.” to improve readability.
Proposed change:-Gets all rates from specific month. +Gets all rates for a specific month.
314-314
: Polish the description for the gold rates daily method.
Rephrase “Returns a gold rate from specific date.” as “Returns a gold rate for a specific date.” for improved clarity.
Proposed change:-Returns a gold rate from specific date. +Returns a gold rate for a specific date.
336-336
: Reword the description for the gold rate before-date method.
Consider changing “Returns a gold rate from before a specific date.” to “Returns a gold rate for the day preceding a specific date.” for greater precision.
Proposed change:-Returns a gold rate from before a specific date. +Returns a gold rate for the day preceding a specific date.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
README.md
(6 hunks)api.md
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- README.md
🧰 Additional context used
🪛 LanguageTool
api.md
[uncategorized] ~3-~3: Possible missing article found.
Context: ...rageRatesService This service provides API for accessing the average rates publish...
(AI_HYDRA_LEO_MISSING_AN)
[uncategorized] ~57-~57: Possible missing article found.
Context: ...s for the prices to be calculated using currency rate applied in the business day before...
(AI_HYDRA_LEO_MISSING_THE)
[typographical] ~189-~189: It appears that a comma is missing.
Context: ...nt in table from the next day. In such case you should not use the getRate($rate)
...
(DURING_THAT_TIME_COMMA)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
api.md (1)
25-31
: Add Language Specifiers to Fenced Code Blocks
Several fenced code blocks that display sample outputs do not currently specify a language. For improved syntax highlighting and to comply with Markdown style guidelines (MD040), please add a language specifier (e.g., use "```text" for output blocks) to the following code block ranges:
• Lines 25–31 (sample output forfromMonth
)
• Lines 77–79 (sample output for `fromDayBefore`)
• Lines 105–110 (first sample output for `getMonthTablesA`)
• Lines 137–143 (second sample output for `getMonthTablesA`)
• Lines 171–178 (sample output for `getMonthTablesB`)
• Lines 218–224 (sample output for trading rates `fromMonth`)
• Lines 247–249 (sample output for `fromEffectiveDay`)
• Lines 273–275 (sample output for `fromTradingDay`)
• Lines 303–308 (sample output for gold rates `fromMonth`)
• Lines 329–331 (sample output for gold rates `fromDay`)
• Lines 353–355 (sample output for gold rates `fromDayBefore`)Also applies to: 77-79, 105-110, 137-143, 171-178, 218-224, 247-249, 273-275, 303-308, 329-331, 353-355
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
25-25: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
api.md
(1 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
api.md
25-25: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
77-77: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
108-108: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
138-138: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
172-172: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
218-218: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
248-248: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
274-274: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
303-303: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
329-329: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
353-353: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
🔇 Additional comments (13)
api.md (13)
1-4
: Clear Introduction for CurrencyAverageRatesService
The introductory lines clearly state the service’s purpose and set the context for the API.
5-32
: Comprehensive Documentation forfromMonth
Method
The section explains the method’s functionality and provides a detailed PHP example for retrieving monthly average rates.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
25-25: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
34-51
: Clear Example forfromDay
Method
The explanation and PHP snippet effectively illustrate how to obtain a dictionary of rates for a specified day.
52-82
: Detailed Explanation forfromDayBefore
Method
The documentation describes the use case very well and includes a PHP example showing how to retrieve rates for the previous business day.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
77-77: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
83-120
: Well-Structured Examples forgetMonthTablesA
Method
Two examples are provided: one for iterating over all tables to get individual rates, and another for retrieving a specific currency rate. This dual example approach enhances clarity.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
108-108: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
121-143
: Clear Usage for Specific Currency Retrieval ingetMonthTablesA
The second example reinforces how to extract a particular currency rate from the monthly tables.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
138-138: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
147-178
: Good Use of Exception Handling ingetMonthTablesB
Method
The example incorporates a try-catch block to handle cases where a currency code might be missing, which is a nice touch to demonstrate error handling.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
172-172: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
196-224
: Effective Example for Trading RatesfromMonth
Method
The PHP example and corresponding sample output clearly demonstrate how to retrieve and iterate over trading rates for an entire month.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
218-218: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
228-252
: Well-DocumentedfromEffectiveDay
Method Example
The example provides a clear guide on fetching rates based on the effective day, which is essential for understanding date-based rate retrieval.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
248-248: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
254-278
: Clear Example for Trading RatesfromTradingDay
Method
The provided example concisely illustrates how to access trading day rates using the corresponding API method.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
274-274: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
280-310
: Informative Example for Gold RatesfromMonth
Method
The detailed example for retrieving gold rates over a month is both informative and easy to follow.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
303-303: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
312-332
: Concise Example for Gold RatesfromDay
Method
The example clearly demonstrates how to obtain the gold rate for a specific date.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
329-329: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
335-357
: Detailed Example for Gold RatesfromDayBefore
Method
This section effectively shows how to fetch the gold rate for the business day preceding the provided date, which is useful for compliance scenarios.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
353-353: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
Summary by CodeRabbit