Skip to content
This repository has been archived by the owner on Sep 21, 2024. It is now read-only.

Compatibility with hangfire beta (1.7.0) #9

Merged
merged 2 commits into from
Nov 29, 2018

Conversation

Olteanu-Mihai
Copy link

Added new option hideYears; the generated cron expression now takes hideSeconds and hideYears into account - i.e. when they are set to true the generated cron will omit seconds and years from the expression, thus making the expression compatible with Hangfire beta

@claudiuconstantin
Copy link
Owner

Thank you for your contribution, will look into it. I think this addresses #4 and #5 too

Copy link
Owner

@claudiuconstantin claudiuconstantin left a comment

Choose a reason for hiding this comment

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

  • the defaults should stay as they currently are, those describe the most common scenario the library is used
  • the /docs folder (used by Github Pages to serve the demo) should be manually updated with the latest app build as this is not done automatically yet (use npm run docs command)
  • when setting hideSeconds: false, hideYears: false we get an error: "Uncaught (in promise): Invalid cron expression, there must be 7 segments" - please double check your changes and ensure no breaking changes are introduced

@Olteanu-Mihai Olteanu-Mihai force-pushed the master branch 3 times, most recently from 57a9177 to 303ffb2 Compare November 28, 2018 10:35
hideSeconds: boolean;
hideSeconds: boolean; //hides the seconds field in the UI

removeSeconds: false; // removes seconds from the CRON expression

Choose a reason for hiding this comment

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

Please make these two members boolean, otherwise you can only assign false to them

@@ -295,5 +295,8 @@
</div>
</div>
</div>
<div clas="row" *ngIf="!state.validation.isValid">

Choose a reason for hiding this comment

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

class instead of clas here

private cronIsValid(cron: string): boolean {
if (cron) {
const cronParts = cron.split(" ");
private cronIsValid(cron: string): void {

Choose a reason for hiding this comment

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

Please note that this method is called on line 230 and a return value is expected there, therefore void is not appropriate. Decide upon returning a value or not and name the method accordingly

@claudiuconstantin claudiuconstantin merged commit fa79627 into claudiuconstantin:master Nov 29, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants