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.

Sign up for GitHub

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

Jump to bottom

added semver official regex pattern #4644

Merged
merged 7 commits into from
Feb 1, 2022
Merged

added semver official regex pattern #4644

merged 7 commits into from
Feb 1, 2022

Conversation

alswang18
Copy link
Contributor

@alswang18 alswang18 commented Jan 30, 2022 •
edited
Loading

resolves #4453

Description

Added support for full semver without breaking support for 2 group versioning

Checklist

  • I have signed the CLA
  • I have run this code in development and it appears to resolve the stated issue
  • This PR includes tests, or tests are not required/relevant for this PR
  • I have updated the CHANGELOG.md and added information about my change

@alswang18 alswang18 requested a review from a team as a code owner January 30, 2022 10:42
@cla-bot
Copy link

cla-bot bot commented Jan 30, 2022

Thanks for your pull request, and welcome to our community! We require contributors to sign our Contributor License Agreement and we don't seem to have your signature on file. Check out this article for more information on why we have a CLA.

In order for us to review and merge your code, please submit the Individual Contributor License Agreement form attached above above. If you have questions about the CLA, or if you believe you've received this message in error, don't hesitate to ping @drewbanin.

CLA has not been signed by users: @alswang18

@b0lle
Copy link

b0lle commented Jan 31, 2022 •
edited
Loading

I tested some cases on regex101.com. I think the regex should not start with "/" as this is no valid pattern in python.

Unfortunately there seems to be no opportunity to extend some test cases :/

@cla-bot
Copy link

cla-bot bot commented Jan 31, 2022

Thanks for your pull request, and welcome to our community! We require contributors to sign our Contributor License Agreement and we don't seem to have your signature on file. Check out this article for more information on why we have a CLA.

In order for us to review and merge your code, please submit the Individual Contributor License Agreement form attached above above. If you have questions about the CLA, or if you believe you've received this message in error, don't hesitate to ping @drewbanin.

CLA has not been signed by users: @alswang18

@b0lle
Copy link

b0lle commented Jan 31, 2022

Great. That should do the trick :)

@cla-bot
Copy link

cla-bot bot commented Jan 31, 2022

Thanks for your pull request, and welcome to our community! We require contributors to sign our Contributor License Agreement and we don't seem to have your signature on file. Check out this article for more information on why we have a CLA.

In order for us to review and merge your code, please submit the Individual Contributor License Agreement form attached above above. If you have questions about the CLA, or if you believe you've received this message in error, don't hesitate to ping @drewbanin.

CLA has not been signed by users: @alswang18

@alswang18
Copy link
Contributor Author

@b0lle Should be better now
image

@alswang18
Copy link
Contributor Author

Also, I signed the CLA over the weekend, wondering why the bot wont recognize me.

@cla-bot
Copy link

cla-bot bot commented Jan 31, 2022

Thanks for your pull request, and welcome to our community! We require contributors to sign our Contributor License Agreement and we don't seem to have your signature on file. Check out this article for more information on why we have a CLA.

In order for us to review and merge your code, please submit the Individual Contributor License Agreement form attached above above. If you have questions about the CLA, or if you believe you've received this message in error, don't hesitate to ping @drewbanin.

CLA has not been signed by users: @alswang18

@cla-bot
Copy link

cla-bot bot commented Jan 31, 2022

Thanks for your pull request, and welcome to our community! We require contributors to sign our Contributor License Agreement and we don't seem to have your signature on file. Check out this article for more information on why we have a CLA.

In order for us to review and merge your code, please submit the Individual Contributor License Agreement form attached above above. If you have questions about the CLA, or if you believe you've received this message in error, don't hesitate to ping @drewbanin.

CLA has not been signed by users: @alswang18

@leahwicz
Copy link
Contributor

@cla-bot check

@cla-bot
Copy link

cla-bot bot commented Jan 31, 2022

Thanks for your pull request, and welcome to our community! We require contributors to sign our Contributor License Agreement and we don't seem to have your signature on file. Check out this article for more information on why we have a CLA.

In order for us to review and merge your code, please submit the Individual Contributor License Agreement form attached above above. If you have questions about the CLA, or if you believe you've received this message in error, don't hesitate to ping @drewbanin.

CLA has not been signed by users: @alswang18

@cla-bot
Copy link

cla-bot bot commented Jan 31, 2022

The cla-bot has been summoned, and re-checked this pull request!

@emmyoop
Copy link
Member

emmyoop commented Jan 31, 2022 •
edited
Loading

@alswang18 it looks like your formatting was a bit off when you signed the CLA. It should be just your username alswang18, no @ or github url. If you sign with that format you should be good to go!

@alswang18
Copy link
Contributor Author

@alswang18 it looks like your formatting was a bit off when you signed the CLA. It should be just your username alswang18, no @ or github url. If you sign with that format you should be good to go!

Just tried it again

@alswang18
Copy link
Contributor Author

@cla-bot check

@cla-bot cla-bot bot added the cla:yes label Jan 31, 2022
@cla-bot
Copy link

cla-bot bot commented Jan 31, 2022

The cla-bot has been summoned, and re-checked this pull request!

gshank
gshank approved these changes Jan 31, 2022
View reviewed changes
Copy link
Contributor

@gshank gshank 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.

@alswang18
Copy link
Contributor Author

@gshank Can we merge this in?

@leahwicz
Copy link
Contributor

@alswang18 could you do one last thing and add an entry in the Changelog file for this change. It would be under section 1.1.0. Also add yourself as a contributor!

@alswang18
Copy link
Contributor Author

Got it, just added @leahwicz

@alswang18 alswang18 requested a review from a team as a code owner January 31, 2022 21:33
@alswang18 alswang18 requested review from ChenyuLInx and VersusFacit and removed request for a team January 31, 2022 21:33
leahwicz
leahwicz reviewed Jan 31, 2022
View reviewed changes
CHANGELOG.md Outdated Show resolved Hide resolved
@alswang18 @leahwicz
Update CHANGELOG.md
6d75c29 
Co-authored-by: leahwicz <[email protected]>
@leahwicz
Copy link
Contributor

Thanks @alswang18 for the contribution!!!

@leahwicz leahwicz closed this Jan 31, 2022
@leahwicz leahwicz reopened this Jan 31, 2022
@leahwicz
Copy link
Contributor

Some GitHub Checks funkiness so closing and reopening to kick them off

@alswang18
Copy link
Contributor Author

@leahwicz sure, when do we merge?

@leahwicz leahwicz merged commit b9867e8 into dbt-labs:main Feb 1, 2022
iknox-fa pushed a commit that referenced this pull request Feb 8, 2022
@alswang18 @iknox-fa
added semver official regex pattern (#4644)
4fefe32 
* added semver official regex pattern

* removed extra character

* added comma

* added contribution

* fixed contribution

* Update CHANGELOG.md

Co-authored-by: leahwicz <[email protected]>

Co-authored-by: leahwicz <[email protected]>

automatic commit by git-black, original commits:
  b9867e8
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gshank gshank gshank approved these changes

@ChenyuLInx ChenyuLInx Awaiting requested review from ChenyuLInx ChenyuLInx was automatically assigned from dbt-labs/core-group

@VersusFacit VersusFacit Awaiting requested review from VersusFacit VersusFacit was automatically assigned from dbt-labs/core-group

@leahwicz leahwicz Awaiting requested review from leahwicz

Assignees
No one assigned
Labels
cla:yes ok_to_test
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Bug] dbt_project.yml does not fully support semantic versioning
6 participants
@alswang18 @b0lle @leahwicz @emmyoop @gshank @jtcohen6