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

For #153 #154

Closed
wants to merge 4 commits into from
Closed

For #153 #154

wants to merge 4 commits into from

Conversation

daniel-dgi
Copy link
Contributor

Removing old ResourceService example.

@@ -1 +1,3 @@
vendor/
composer.lock
composer.phar
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From personal experience, including the lock files always ends with some sort of confusion over why things won't update when people are testing changes.

I understand this is contrary to what stack overflow says.

It's bitten me before many many times, and I make it a habit not to. With the understanding that I need to know what I'm doing if deploying straight from the git repo on a production environment.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On second thought, maybe I'm being too biased. Let's try it the 'correct' way. But if it turns out to be a serious PITA I'll issue a PR to get rid of them.

@whikloj
Copy link
Member

whikloj commented Feb 18, 2016

So we are going to have a ResourceService that is one index.php file?

@daniel-dgi
Copy link
Contributor Author

Nope. I just renamed everything so there's only ResourceService. It is split apart with providers.

@whikloj
Copy link
Member

whikloj commented Feb 18, 2016

Oh okay, I get it now.

@daniel-dgi
Copy link
Contributor Author

Yeah, that PR is muddy b/c of how it views renaming and deletions. No worries.

@ruebot
Copy link
Member

ruebot commented Feb 18, 2016

@daniel-dgi did you do a git mv on this or just a mv?

@daniel-dgi
Copy link
Contributor Author

plain ol' mv

@ruebot
Copy link
Member

ruebot commented Feb 18, 2016

Ok, that makes sense then.

@ruebot ruebot added this to the Community Sprint - 04 milestone Feb 19, 2016
@ruebot
Copy link
Member

ruebot commented Feb 19, 2016

@daniel-dgi this isn't the refactor stuff for #148, right?

@daniel-dgi
Copy link
Contributor Author

No, but it's something that needs to go because it's SUPER confusing to have an example and the real implementation side by side with similar names. Just a quick cleanup I did before I started the refactor.

@ruebot
Copy link
Member

ruebot commented Feb 19, 2016

Cool.

@whikloj, @DiegoPino how are y'all on this one now? Good to merge?

@ruebot
Copy link
Member

ruebot commented Feb 19, 2016

@daniel-dgi gotta rebase now since I merged #152

@DiegoPino
Copy link
Contributor

@daniel-dgi, i don't want to be a pain, but could you re-do this using git mv?
git mv -f oldfolder newfolder (the f is force to overwrite)
and then update the properties. It's less history messy and in the long term it gets recorded as such.
Thanks

@daniel-dgi
Copy link
Contributor Author

I'll just close and re-open, trying with git mv to see if it's any better.

@daniel-dgi
Copy link
Contributor Author

Superceded by #155

@daniel-dgi daniel-dgi closed this Feb 22, 2016
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 this pull request may close these issues.

4 participants