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

Relax GitURL schema: allow no ref - default to master #80

Closed
wants to merge 1 commit into from

Conversation

kptdobe
Copy link
Contributor

@kptdobe kptdobe commented Mar 27, 2019

Second run for: #71

This should unlock the backward compatibility.

@ghost
Copy link

ghost commented Mar 27, 2019

There were the following issues with this Pull Request

  • Commit: 6839df6
    • ✖ message may not be empty
    • ✖ type may not be empty

You may need to change the commit messages to comply with the repository contributing guidelines.


🤖 This comment was generated by commitlint[bot]. Please report issues here.

Happy coding!

@kptdobe kptdobe requested review from tripodsan and trieloff and removed request for tripodsan March 27, 2019 15:44
@kptdobe kptdobe self-assigned this Mar 27, 2019
@kptdobe kptdobe added the bug Something isn't working label Mar 27, 2019
@codecov
Copy link

codecov bot commented Mar 27, 2019

Codecov Report

Merging #80 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #80   +/-   ##
=======================================
  Coverage   98.04%   98.04%           
=======================================
  Files          14       14           
  Lines        1022     1022           
  Branches      226      226           
=======================================
  Hits         1002     1002           
  Misses         20       20

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1706f48...6839df6. Read the comment docs.

@tripodsan
Copy link
Contributor

-1. we said that refs are mandatory. we should not be backward compatible to a non existing previous version.

@kptdobe
Copy link
Contributor Author

kptdobe commented Mar 28, 2019

@kptdobe kptdobe closed this Mar 28, 2019
@kptdobe kptdobe deleted the sequence-strains-2 branch March 28, 2019 07:44
@tripodsan
Copy link
Contributor

actually, I'm not sure about this anymore. I think the reasoning was, since we defined a default via schema, it acts like it would be defined in the config. I think we only said, that we should not have implicit defaults via code.

@tripodsan tripodsan restored the sequence-strains-2 branch April 1, 2019 00:44
@tripodsan tripodsan reopened this Apr 1, 2019
@tripodsan
Copy link
Contributor

closing as WONT'F FIX see #82

@tripodsan tripodsan closed this May 13, 2019
@tripodsan tripodsan deleted the sequence-strains-2 branch July 11, 2019 02:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants