Skip to content

Conversation

thaianhtaivn
Copy link

Already tested function with platformIO & ESP8266.

@CLAassistant
Copy link

CLAassistant commented Jan 12, 2023

CLA assistant check
All committers have signed the CLA.

@per1234 per1234 added type: enhancement Proposed improvement topic: code Related to content of the project itself labels Jan 12, 2023
@per1234
Copy link
Contributor

per1234 commented Jan 12, 2023

@thaianhtaivn
Copy link
Author

Hello @per1234 ,
Because we usually use it, can we merge these functions to master?
Thank you!

@per1234
Copy link
Contributor

per1234 commented Jan 12, 2023

Hi @thaianhtaivn. Thanks for your pull request. My role in this project is somewhat janitorial in nature. I don't make the decisions about which pull requests are merged or not. I commented to provide some supplemental information that might be of use to the reviewers.

Regards,
Per

Copy link

@lukaszgieraltowski lukaszgieraltowski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good job. I'd suggest to add getFormattedDateTime using new methods. Analogously to getFormattedTime.

return (this->getEpochTime() % 60);
}
int NTPClient::getYears() const {
time_t rawtime = this->getEpochTime();

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use two spaces for indentation instead of four.

int getHours() const;
int getMinutes() const;
int getSeconds() const;
int getYears() const;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use two spaces for indentation instead of tab.

@lukaszgieraltowski
Copy link

@thaianhtaivn Actually this was already implemented #94 but not merged :/

@thaianhtaivn thaianhtaivn closed this by deleting the head repository Mar 29, 2023
@per1234 per1234 added the conclusion: duplicate Has already been submitted label Mar 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
conclusion: duplicate Has already been submitted topic: code Related to content of the project itself type: enhancement Proposed improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants