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

[deps] min phan version #4168

Closed
wants to merge 1 commit into from
Closed

[deps] min phan version #4168

wants to merge 1 commit into from

Conversation

Jkat
Copy link
Contributor

@Jkat Jkat commented Dec 3, 2018

Anyone know if this will break something? I think as our systems are getting newer, we're no longer installing low versions of ast which phan 0.12.x requires...also isn't phan 0.12.x a bit old?

@johnsaigle
Copy link
Contributor

@ridz1208 Can you comment here? I think I remember you mentioning this being a problem earlier. Will this fix it?

@johnsaigle johnsaigle added Testing PR contains test plan or automated test code (or config files for Travis) [branch] minor labels Dec 3, 2018
@johnsaigle
Copy link
Contributor

johnsaigle commented Dec 3, 2018

Anyone know if this will break something?

This PR seems to have phan errors. I guess updating the version introduces more checks that we are not passing.

Also I think it's probably safe to update this to 1.0 since that's what major is using.

@Jkat
Copy link
Contributor Author

Jkat commented Dec 3, 2018

ah cool, just found the #4114 thanks

are we documenting the version for the ast dependency somewhere? if im not mistaken we're supposed to install the ast dependency externally(as in not part of the standard loris install process), which seems to require certain versions depending on what version of phan we're using.

@johnsaigle
Copy link
Contributor

johnsaigle commented Dec 3, 2018

I don't think it's documented. It's not yet part of the standard LORIS install though I have amended this in the proposed (and probably cursed) update script back in May.

My solution there was to install it using apt though I know @ridz1208 has used pecl with some difficulty. Haven't looked into CentOS at this time though this is obviously a priority.

This in general is part of a series of related efforts/larger discussion of installation/bootstrapping/updating. From what I understand this is to be discussed in roadmap meetings. Maybe @driusan or @ridz1208 could shed some light here but I believe these issues are still unresolved.

@Jkat
Copy link
Contributor Author

Jkat commented Dec 3, 2018

cool cool, and just a small note for context, i ran into this issue on CentOS 7

@driusan
Copy link
Collaborator

driusan commented Dec 3, 2018

the ast extension isn't required for LORIS, it's just required for development. It can't be installed by composer because it's a PHP extension, not a PHP package.

@Jkat
Copy link
Contributor Author

Jkat commented Dec 3, 2018

are we documenting the version for the ast dependency somewhere for developers? if im not mistaken developers are supposed to install the ast dependency externally(as in not part of the standard loris install process), which seems to require certain versions depending on what version of phan is being used.
(sorry wasn't being clear earlier)

@johnsaigle
Copy link
Contributor

@Jkat I don't we are but we should be. Maybe in the process of this wiki revamp we can include post-install notes for developer instances.

@driusan
Copy link
Collaborator

driusan commented Dec 3, 2018

That's a good question.. @ridz1208 is the php-ast version documented anywhere in the wiki revamp.

@ridz1208
Copy link
Collaborator

ridz1208 commented Dec 5, 2018

@driusan shouldnt it be documented somewhere in the readme ? since its a dependency (even if its for developers... it could be in a devs section)

@Jkat
Copy link
Contributor Author

Jkat commented Dec 5, 2018

(this PR can be closed btw once we decide on a documentation solution)

@davidblader davidblader removed Add to Release Notes PR change should be highlighted in Release notes (important security, features and bugfixes) labels Dec 7, 2018
@johnsaigle
Copy link
Contributor

@ridz1208 Could you give a Wiki/documentation update in a LORIS meeting sometime soon?

@driusan
Copy link
Collaborator

driusan commented Dec 10, 2018

#4172 seems like a more appropriate place to have this discussion rather than keeping this PR open forever.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Testing PR contains test plan or automated test code (or config files for Travis)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants