Skip to content
New issue

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

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

Already on GitHub? # to your account

Standardize TCK formatting #316

Merged
merged 14 commits into from
Aug 11, 2023
Merged

Conversation

KyleAure
Copy link
Contributor

@KyleAure KyleAure commented Aug 1, 2023

Run the maven-checkstyle-plugin on the TCK and ensure formatting between contributors.

The current TCK which was mostly copied from the Platform-TCK has a mix of tabs and spaces, along with a bunch of other non-standard formatting issues.

Also had to fix some bugs in the signature testing logic, which should all be resolved by the last two commits.

@KyleAure KyleAure added the TCK label Aug 1, 2023
@KyleAure KyleAure self-assigned this Aug 1, 2023
Copy link
Member

@mswatosh mswatosh left a comment

Choose a reason for hiding this comment

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

Looks good, I opened an issue for refactoring CronTrigger

* @param min minute
* @param h hour
* @param d day
* @param l ??
Copy link
Member

Choose a reason for hiding this comment

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

int d = Arrays.binarySearch(daysOfMonth, dayOfMonth);
int l = Arrays.binarySearch(daysOfMonth, dayOfMonth - lastDayOfMonth - 1);

I did some digging, and from what I see, d isn't (exactly) the day, it's the index of the day in the daysOfMonth array. And l is the offset from the last day of the month in the daysOfMonth array.

Copy link
Member

Choose a reason for hiding this comment

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

I opened #318 to address these, so they don't need to be addressed here.

* @param d day
* @param l ??
* @param dayOfMonth day of month
* @param m minute
Copy link
Member

Choose a reason for hiding this comment

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

int m = Arrays.binarySearch(months, time.getMonthValue());

like above, m is the index of the month in the months array. None of these need to be updated now, but we may want to open an issue to refactor some of this for maintainability

@KyleAure KyleAure merged commit 1158396 into jakartaee:main Aug 11, 2023
@KyleAure KyleAure deleted the style-and-formatting branch August 11, 2023 21:08
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants