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

Framework validation issue #105

Closed
keith opened this issue Apr 7, 2014 · 13 comments
Closed

Framework validation issue #105

keith opened this issue Apr 7, 2014 · 13 comments

Comments

@keith
Copy link
Member

keith commented Apr 7, 2014

Frameworks should be allowed to have more than just letters and numbers. As it currently stands those are the only characters allowed although some frameworks have - in the name. This can be fixed by changing the regex to /[^a-zA-Z\-\d]/ and writing a test or two.

@keith keith added the d1:easy label Apr 7, 2014
@keith
Copy link
Member Author

keith commented Apr 7, 2014

This issue also exists for libraries from here libraries should also be able to have - and + (stdc++?)

@keith
Copy link
Member Author

keith commented Apr 7, 2014

These should both also allow underscores which could simplify the regex to /[^\w\-\+]/

@keith
Copy link
Member Author

keith commented Apr 7, 2014

There may be a larger issue with this. I've seen a few specs including specific versions I assume with something like z.1 or std++.6.0.9 which wouldn't work with this current plan.

@keith
Copy link
Member Author

keith commented Apr 7, 2014

curl.OSX although this example could probably switch to vendored_libraries

@keith
Copy link
Member Author

keith commented Apr 7, 2014

Another weird one: Geoloqi-$(CONFIGURATION)

@keith
Copy link
Member Author

keith commented Apr 7, 2014

At this point all the libraries that showed up because of this stricter check and were actually broken have been fixed. All the rest are currently correct with one of the above issues.

@keith keith added t2:defect and removed d1:easy labels Apr 7, 2014
@fabiopelosin
Copy link
Member

To recap tests should accept:

  • stdc++
  • curl.OSX
  • z.1
  • Geoloqi-$(CONFIGURATION)

@keith
Copy link
Member Author

keith commented Apr 7, 2014

👍 sorry kept running into new ones.

@keith
Copy link
Member Author

keith commented Apr 7, 2014

I think the best way to validate this would just be to check for lib in the beginning and the dylib at the end. I was hoping we could verify with dot's but that obviously wouldn't work either.

@joshkalpin
Copy link
Member

Just to summarize, frameworks just need the regex modified but libraries need to be reverted?

@keith
Copy link
Member Author

keith commented Apr 8, 2014

I believe so. Frameworks shouldn't be able to specify versions like that (I don't think). I didn't see any frameworks trying to do that in the list I was going over yesterday.

@fabiopelosin
Copy link
Member

The main purpose of the regex was to prevent the accidental specification of:

spec.frameworks = 'z, x'

So it shouldn't be completely reverted. I also think that we should add the examples above to the specs. Finally, I am of the opinion that we should not be over restrictive, and although not framework uses versions like that it is still a valid name and should be allowed.

@fabiopelosin
Copy link
Member

7b44d3c

Ashton-W pushed a commit to Ashton-W/Core that referenced this issue Nov 2, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants