Skip to content
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

Configurable date utility or native for date validators #69

Closed
snewcomer opened this issue Dec 2, 2019 · 11 comments · Fixed by #100
Closed

Configurable date utility or native for date validators #69

snewcomer opened this issue Dec 2, 2019 · 11 comments · Fixed by #100

Comments

@snewcomer
Copy link
Collaborator

As apps have more options available to them (luxon, date-fns) and moment-core recommends using luxon, we could make this configurable and fallback to browser native. We can also include some of the pitfalls of parsing strings with the Date constructor so ppl know if they might have any issues.

This would allow this validator to have broader use. I'd be happy to do the work.

ref adopted-ember-addons/ember-changeset-validations#208

@jherdman
Copy link

jherdman commented May 6, 2020

Supporting date-fns would be nice to have too. I'm able to help on that front.

@snewcomer
Copy link
Collaborator Author

@jherdman I'm not sure what library you need for this. For e-c-v, we have a basic date validator. We need to improve it with comparing Date as a String. That is probably good enough (w/o date-fns). But if you had another opinion regarding use of a standardized library, I'm all ears (and would love for us to fix in this library)

adopted-ember-addons/ember-changeset-validations#204 (comment)

@snewcomer
Copy link
Collaborator Author

@jherdman Do you have write and publish access to this repo?

@jherdman
Copy link

Sadly I do not.

@snewcomer
Copy link
Collaborator Author

@offirgolan 👋 Do you think James or I could get contributor + publish rights? There are some changes needed here to support embroider - specifically dynamic resolution + compat with ember-changeset-validations. The only other option I might have is to copy and paste the files we need in e-c-v direction. But hopefully we can continue maintaining this package!

https://github.com/offirgolan/ember-validators/blob/4ec87cd93dfcdeaf24dafd2a7198f1649d76a040/addon/index.js#L6

@maxymczech
Copy link

maxymczech commented May 25, 2021

@snewcomer In the meanwhile, how would one use ember-cp-validations with embroider? I am trying to use the following configurations, but it causes Assertion Failed: Validator not found of type: format error:

    packageRules: [{
      package: 'ember-validators',
      addonModules: {
        'index.js': {
          dependsOnModules: [
            'alias',
            'base',
            'belongs-to',
            'collection',
            'confirmation',
            'date',
            'dependent',
            'ds-error',
            'exclusion',
            'format',
            'has-many',
            'inclusion',
            'inline',
            'length',
            'messages',
            'number',
            'presence'
          ].map(validator => `ember-cp-validations/validators/${validator}`)
        },
      },
    }],

@offirgolan
Copy link
Collaborator

Apologies for being MIA, I've unfortunately been out of the ember scene for quite some time. @snewcomer I've added you as a collaborator and given you publish access on npm.

@snewcomer
Copy link
Collaborator Author

@offirgolan Thank you and completely understandable! One last issue is we will need the "Required" CI check turned off in order for us to migrate from travis to github actions. I think this is in the "Settings" tab, which I don't have access to.

I can't see the travis run for some reason. Travis still is free with Linux builds but it seems sane to migrate to Actions for better support.

#92

@offirgolan
Copy link
Collaborator

@snewcomer I've completely removed the travis.ci webhook, hopefully that should be enough to let you fully migrate to GH actions.

@snewcomer
Copy link
Collaborator Author

Hi @offirgolan ! Sry to bother again. It looks like it is in Settings > Branch > Branch protection rules as well that is requiring Travis.

https://docs.github.com/en/github/administering-a-repository/defining-the-mergeability-of-pull-requests/managing-a-branch-protection-rule

Screen Shot 2021-06-02 at 9 30 19 AM

@offirgolan
Copy link
Collaborator

ah! my bad, I thought just disconnecting the webhook would do that. Should be good to go now!

This was referenced Jun 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants