CB-14140 WIP fs-extra instead of shelljs #14

brody4hire · 2018-05-29T16:59:31Z

Platforms affected

All

What does this PR do?

Introduce usage of fs-extra and isolate shelljs usage, as a first step towards completely dropping shelljs dependency as discussed in apache/cordova-common#21 (comment) and apache/cordova-discuss#69 (comment)

What testing has been done on this change?

Changes to index.js pass existing spec suite

Checklist

~~Reported an issue in the JIRA database~~
~~Commit message follows the format: "CB-3232: (android) Fix bug with resolving file paths", where CB-xxxx is the JIRA ID & "android" is the platform affected.~~
~~Added automated test coverage as appropriate for this change.~~

FUTURE TODO

As discussed in apache/cordova-common#21 (comment) and apache/cordova-discuss#69 (comment) it is desired to complete drop dependency on shelljs. My first attempt to replace shell.cp call with fs-extra call did not pass the existing test suite. If someone else can resolve this issue I would be happy to amend this PR or maybe even drop it in favor of another solution.

dpogue

The usage of fs-extra.copySync is a bit different than shelljs.copy, particularly around directories.

fs.copySync always copies the contents of a directory, so you never need /* at the end of paths. It will also automatically ensure that the destination directory exists.

So if you want to copy template/www/* to project/www/:

shelljs.copy('-R', path.join(template, 'www', '*'), path.join(project, 'www'));

fs.copySync(path.join(template, 'www'), path.join(project, 'www'));

Note that if you want to copy an entire directory, you'll need to make sure the destination path is set up properly:

shelljs.copy('-R', path.join(template, 'www'), project);

fs.copySync(path.join(template, 'www'), path.join(project, 'www'));

dpogue · 2018-05-29T17:32:07Z

index.js

+ * @param  {string} dst for copying
+ * @return No return value
+ */
+function copyFile (src, dst) {


You should be able to safely use fs.copySync(src, dst) in the two places this is getting called

You should be able to safely use fs.copySync(src, dst) in the two places this is getting called

With the following change:

--- a/index.js +++ b/index.js @@ -235,7 +235,7 @@ module.exports = function (dir, optionalId, optionalName, cfg, extEvents) { copyContentsIfNotExists(stockAssetPath('hooks'), path.join(dir, 'hooks')); var configXmlExists = projectConfig(dir); // moves config to root if in www if (!configXmlExists) { - copyFile(stockAssetPath('config.xml'), path.join(dir, 'config.xml')); + fs.copySync(stockAssetPath('config.xml'), path.join(dir, 'config.xml')); } } catch (e) { if (!dirAlreadyExisted) { @@ -429,7 +429,7 @@ function linkFromTemplate (templateDir, projectDir) { // if template/www/config.xml then copy to project/config.xml copyDst = path.join(projectDir, 'config.xml'); if (!fs.existsSync(copyDst) && fs.existsSync(copySrc)) { - copyFile(copySrc, projectDir); + fs.copySync(copySrc, projectDir); } }

I get the following result:

> [email protected] jasmine /Users/brodybits/Documents/cbwork/cordova-create > jasmine spec/create.spec.js Started .............F....... Failures: 1) create end-to-end when --link-to is provided when passed www folder should not move www/config.xml, only copy and update Message: Error: EPERM: operation not permitted, unlink '/var/folders/0j/4p0hqtr54914lbts82d2z9vh0000gn/T/cordova-create-tests-jR6vV0/TestBase' Stack: error properties: Object({ errno: -1, syscall: 'unlink', code: 'EPERM', path: '/var/folders/0j/4p0hqtr54914lbts82d2z9vh0000gn/T/cordova-create-tests-jR6vV0/TestBase' }) Error: EPERM: operation not permitted, unlink '/var/folders/0j/4p0hqtr54914lbts82d2z9vh0000gn/T/cordova-create-tests-jR6vV0/TestBase' at Object.fs.unlinkSync (fs.js:1029:3) at mayCopyFile (/Users/brodybits/Documents/cbwork/cordova-create/node_modules/fs-extra/lib/copy-sync/copy-sync.js:59:8) at onFile (/Users/brodybits/Documents/cbwork/cordova-create/node_modules/fs-extra/lib/copy-sync/copy-sync.js:53:47) at getStats (/Users/brodybits/Documents/cbwork/cordova-create/node_modules/fs-extra/lib/copy-sync/copy-sync.js:47:39) at startCopy (/Users/brodybits/Documents/cbwork/cordova-create/node_modules/fs-extra/lib/copy-sync/copy-sync.js:37:10) at Object.copySync (/Users/brodybits/Documents/cbwork/cordova-create/node_modules/fs-extra/lib/copy-sync/copy-sync.js:32:10) at linkFromTemplate (/Users/brodybits/Documents/cbwork/cordova-create/index.js:432:12) at /Users/brodybits/Documents/cbwork/cordova-create/index.js:229:21 at _fulfilled (/Users/brodybits/Documents/cbwork/cordova-create/node_modules/q/q.js:854:54) at /Users/brodybits/Documents/cbwork/cordova-create/node_modules/q/q.js:883:30 21 specs, 1 failure Finished in 3.952 seconds

Any clues?

You should be able to safely use fs.copySync(src, dst) in the two places this is getting called

It actually works for me in one place, just pushed the change. The following change still gives me the EPERM: operation not permitted error:

--- a/index.js +++ b/index.js @@ -429,7 +429,7 @@ function linkFromTemplate (templateDir, projectDir) { // if template/www/config.xml then copy to project/config.xml copyDst = path.join(projectDir, 'config.xml'); if (!fs.existsSync(copyDst) && fs.existsSync(copySrc)) { - copyFile(copySrc, projectDir); + fs.copySync(copySrc, projectDir); } }

brody4hire · 2018-05-29T19:02:59Z

@dpogue I was able to use fs.copySync in almost all places as you suggested, just pushed the changes.

However the following change persistently fails with EPERM: operation not permitted errors:

diff --git a/index.js b/index.js
index 9777240..d8f34d8 100644
--- a/index.js
+++ b/index.js
@@ -337,7 +337,7 @@ function copyTemplateFiles (templateDir, projectDir, isSubDir) {
         // Copy each template file after filter
         for (var i = 0; i < templateFiles.length; i++) {
             copyPath = path.resolve(templateDir, templateFiles[i]);
-            copyFolderWithSymlinks(copyPath, projectDir);
+            fs.copySync(copyPath, projectDir);
         }
     }
 }

For example:

1) create end-to-end should successfully run without template and use default hello-world app
  Message:
    Error: EPERM: operation not permitted, unlink '/var/folders/0j/4p0hqtr54914lbts82d2z9vh0000gn/T/cordova-create-tests-cVce6b/TestBase'
  Stack:
    error properties: Object({ errno: -1, syscall: 'unlink', code: 'EPERM', path: '/var/folders/0j/4p0hqtr54914lbts82d2z9vh0000gn/T/cordova-create-tests-cVce6b/TestBase' })
    Error: EPERM: operation not permitted, unlink '/var/folders/0j/4p0hqtr54914lbts82d2z9vh0000gn/T/cordova-create-tests-cVce6b/TestBase'
        at Object.fs.unlinkSync (fs.js:1029:3)
        at mayCopyFile (/Users/brodybits/Documents/cbwork/cordova-create/node_modules/fs-extra/lib/copy-sync/copy-sync.js:59:8)
        at onFile (/Users/brodybits/Documents/cbwork/cordova-create/node_modules/fs-extra/lib/copy-sync/copy-sync.js:53:47)
        at getStats (/Users/brodybits/Documents/cbwork/cordova-create/node_modules/fs-extra/lib/copy-sync/copy-sync.js:47:39)
        at startCopy (/Users/brodybits/Documents/cbwork/cordova-create/node_modules/fs-extra/lib/copy-sync/copy-sync.js:37:10)
        at Object.copySync (/Users/brodybits/Documents/cbwork/cordova-create/node_modules/fs-extra/lib/copy-sync/copy-sync.js:32:10)
        at copyTemplateFiles (/Users/brodybits/Documents/cbwork/cordova-create/index.js:340:16)
        at /Users/brodybits/Documents/cbwork/cordova-create/index.js:223:21
        at _fulfilled (/Users/brodybits/Documents/cbwork/cordova-create/node_modules/q/q.js:854:54)
        at /Users/brodybits/Documents/cbwork/cordova-create/node_modules/q/q.js:883:30

I am starting to suspect an issue with how fs-extra deals with symbolic links, will investigate when I get a chance.

I hope it would be possible to include these changes, since they are needed for me to rebase and rework #8 for CB-12397 without the undesired use of shelljs as discussed in apache/cordova-common#21 (comment) and apache/cordova-discuss#69 (comment). I would be happy to rebase or squash these changes if needed.

brody4hire · 2018-05-29T19:49:38Z

I just did one more bit of cleanup and squashed down to two commits:

use fs-extra in package.json and index.js, with shelljs only used in one place, passes existing spec suite
update spec to completely migrate from shelljs to fs-extra

As I said before there is still one place where I could not replace shelljs without failures. I continue to remain hopeful that these changes can be reviewed and integrated so that I can update #8 to use fs-extra instead of shelljs function call.

brody4hire · 2018-05-29T21:41:25Z

Sorry guys for the bother, these changes will be part of a bigger PR for CB-12397.

raphinesse · 2018-05-29T21:43:08Z

What's the matter? I think this should be its own PR.

brody4hire · 2018-05-29T21:51:40Z

What's the matter? I think this should be its own PR.

I would like to make a couple more updates before rebasing the change in #8:

use const instead of var
rework changes in ~~8e7ad89~~ 53a29ba (test updates before CB-12397 fix)

I was thinking it might be easiest to contribute all in a single cb-12397-updates-before-fix PR before issuing the updated change in #8. One less merge to wait for.

I would also be happy to reopen to keep this change by itself if you think we can get it integrated within the next day or so.

raphinesse · 2018-05-29T22:02:48Z

I see. I can't promise anything about integration, since I cannot merge anything yet and wouldn't want to rush anything either.

I just made some changes to this branch of yours to make it work without shelljs. I'm about to review it right now, but all in all it looks good to me.

Maybe just rename this PR and build your test updates on top of it? And I must say I'd prefer you save the var to const for a different day. Even if it bugs me too, but it's really not that urgent. I'd rather start a separate ES201x-ify initiative later.

brody4hire · 2018-05-29T22:08:15Z

@raphinesse I can think of several ways forward:

you post the changes here and I can include with your name & email in the author field
you post the changes to a special branch and I push them to the branch
you raise a new PR with shelljs completely removed

Please let me know which way you would like to proceed. Reopening for now.

raphinesse · 2018-05-29T22:34:21Z

I filed a PR against your PR branch: brody4hire#1. I think if you merge that, my changes should appear here 🤔

brody4hire · 2018-05-29T23:13:33Z

Thanks @raphinesse, the changes you contributed in brody4hire#1 look really good in general and pass the spec test suite on both my mac and Windows PC. I have the following questions:

The changes pass the spec test suite whether or not the changes you proposed in eec80ce to fix the destination path are included. I wonder if this is because fs.copySync is able to deal with this difference or if we may be missing something in the test suite?

How much should I squash the changes (if at all)?

In general I would favor having one commit with the source changes, shown to pass the existing spec suite, and another commit with the spec test changes, would be open for input. I will definitely include @raphinesse as co-author (thanks for the link).

dpogue · 2018-05-29T23:22:25Z

If you merge/rebase/squash the PR to this branch using the GitHub interface, it should keep the author info intact without you needing to do anything extra 🙂

dpogue · 2018-05-29T23:23:34Z

As for splitting things up, I'd be inclined to squash this whole PR to a single commit when merging to master, so I don't think it matters how many commits you add to the branch.

raphinesse · 2018-05-29T23:25:35Z

I'm fine with having two commits. Seeing the existing tests pass is a valid point. Given the current set of changes I'd leave it at that.

I think it should successfully run with www folder as the template passes without my change since the www folder is added later from cordova-app-hello-world if it is not present. Thus the error is masked and the result still passes checkConfigXml. It's probably similar for the other change. Sorry if this turns out to be wrong but I don't have time to investigate further, right now.

I have to call it a day/night now. I give this my go with my changes included.

brody4hire · 2018-05-30T00:06:03Z

I just pushed the changes in 2 commits, naming myself and @raphinesse as co-authors. I would side with @raphinesse in favor of getting this merged in 2 commits but not so strongly in this case.

I am becoming a bit concerned about the statement by @raphinesse:

I think it should successfully run with www folder as the template passes without my change since the www folder is added later from cordova-app-hello-world if it is not present. Thus the error is masked and the result still passes checkConfigXml.

In general I would rather have spec verify the existing behavior as it may be affected before applying the changes discussed here. I would really like to avoid risk breaking existing behavior if we can avoid it. I will take another look.

raphinesse · 2018-05-30T09:15:26Z

@brodybits I updated raphinesse:introduce-fs-extra-usage with a failing test for one of my changes

brody4hire · 2018-05-30T10:08:12Z

@brodybits I updated raphinesse:introduce-fs-extra-usage with a failing test for one of my changes

0cd13c1 - looks like a nice find

I will probably need a few hours to look at this one, thinking it would be good to update spec to check for this find and a few more artifacts before merging this change.

P.S. +1 for 89211c2 to fix the test error message

brody4hire · 2018-05-30T19:37:09Z

I have updated the changes with a better check for the www artifacts, along the lines of the test in 0cd13c1 proposed by @raphinesse. I have verified that both 68d4c9b (replace shelljs in index.js only) and 92a56ed (shelljs also completely replaced in spec & package.json) pass on both mac and Windows.

The proposed changes are now in 5 commits and I would prefer to keep it that way. (I would also be fine if we squash the first 3 commits for spec updates and keep the last 2 commits separate.)

P.S. The changes include a much simpler fix for the toExist matcher.

raphinesse

I left some remarks, but these might be a matter of taste.

Thanks @brodybits! For me, this is good to go 👍

PS: I would squash commits 2&3 and 4&5 but to be frank, I don't really care.

raphinesse · 2018-05-30T19:58:26Z

spec/create.spec.js

+        expect(path.join(project, 'www', '.gitignore')).not.toExist();
+
+        // Check that .npmignore does not exist inside of www
+        expect(path.join(project, 'www', '.npmignore')).not.toExist();


I would remove all mentions of .npmignore from the specs. I think we should really treat it like any other file. To me, these explicit checks convey that we are handling these files specially.

I would personally favor keeping this check to keep the related behavior as clear as possible.

But our intended behavior is Treat it like any other file. So if a template wanted to define a .npmignore, it could. IMO, the only tests related to .npmignore (if any) should be to check that it is preserved in the project if we have one in the template (or a .gitignore that is being transformed to one).

That being said, this is no show stopper for me.

raphinesse · 2018-05-30T20:01:59Z

spec/helpers.js

@@ -84,7 +83,7 @@ beforeEach(function () {
                    var result = {};
                    result.pass = fs.existsSync(testPath);

-                    if (result.pass) {
+                    if (!result.pass) {


That's a shorter diff, but I find the code to be less clear.

I am starting to agree with you, will do as you had proposed in 0cd13c1.

brody4hire · 2018-05-30T20:28:39Z

@raphinesse I will rework the first commit to do as you had proposed in 0cd13c1, would like to keep the check for .npmignore.

I will definitely squash the first 3 commits with the spec updates. For the last 2 I am undecided whether or not to squash them together. I think we discussed before keeping them separate to show that the changes to index.js pass the existing spec suite, not sure how important it really is (very few changes needed to migrate spec from shelljs to fs-extra).

raphinesse · 2018-05-30T20:36:40Z

I'd keep the first commit separate, since it's an actual fix.

Regarding the last two: I think having been able to see the original specs pass was nice during development, but we don't need it in the history.

(some redundant test code removed) Co-authored-by: Christopher J. Brody <[email protected]> Co-authored-by: Raphael von der Grün <[email protected]>

Co-authored-by: Christopher J. Brody <[email protected]> Co-authored-by: Raphael von der Grün <[email protected]>

brody4hire · 2018-05-30T21:12:02Z

Now squashed down to 3 commits as discussed, first commit with the toExists message fix as originally proposed by @raphinesse (with slight rewording of commit message). @raphinesse it would be great if you can give a final approval for this. I think we both want these commits to be kept as 3 commits in the final merge.

P.S. @raphinesse I see your point that we should not consider .npmignore to be a special artifact in spec, still not convinced though. IMHO anything that could possibly go wrong or otherwise prove to be quirky should be covered in testing to get the actual behavior as known as much as practical. One less thing that could pop up someday when we least expect it:)

raphinesse

LGTM. And yes, I would merge these commits without squashing.

I could merge, I just don't know how much approval one should await before merging 🤔
@dpogue Are you fine with this getting merged?

XXX TODO: USE fs-extra function instead, XXX WAITING FOR apache#14 to be merged NOTE: This implementation is a workaround for the npm .gitignore/.npmignore behavior discussed in: - npm/npm#1862 - npm/npm#3763 - npm/npm#7252

brody4hire · 2018-05-31T20:58:13Z

Another thing is that I would favor introducing this change in the next major release. While I do not expect this change to break anything I cannot be 100% sure. See also apache/cordova-common#21 (comment).

raphinesse · 2018-05-31T21:01:30Z

I think if this is too dangerous to release in a minor version, then master is already on track of a new major version after my latest cleanup.

raphinesse · 2018-06-03T14:46:26Z

Given the delay of the #8 I'm giving this another overhaul to remove any related changes so that we can merge this. Please don't merge until further notice.

raphinesse · 2018-06-03T21:32:38Z

Waiting for #16 to be merged so this can be rebased onto it

raphinesse · 2018-06-04T09:26:44Z

Well, actually this does not really depend on #16 so we could at least reduce this to the fs-extra commit. Can you do this @brodybits?

The fs-extra part is fine by me. Would like one other approval before merging.

brody4hire · 2018-06-04T11:17:05Z

Hmm... To be honest I would rather see the test changes integrated first, including the .giignore and .npmignore checks that I had proposed. On Jun 4, 2018 5:26 AM, "Raphael von der Grün" <[email protected]> wrote: Well, actually this does not really depend on #16 <#16> so we could at least reduce this to the fs-extra commit. Can you do this @brodybits <https://github.com/brodybits>? The fs-extra part is fine by me. Would like one other approval before merging. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#14 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABfNUHhovsJoHG0LYkg-sAEztp7JrCSEks5t5P1VgaJpZM4UR3xM> .

raphinesse · 2018-06-04T12:54:52Z

@brodybits I heard you and I still disagree with you on the *ignore tests. Could we please concentrate on getting the changes merged that we agree on and keep the unrelated changes we disagree on out of it? I have a lot of other changes in the making, that would benefit from being built on top of this and #16.

As soon as #16 is merged you can create a separate PR to discuss your *ignore tests, but I don't want to bargain over a package deal here. And please do believe me that the whole .gitignore topic is very high priority for me too.

brody4hire · 2018-06-04T13:11:40Z

@raphinesse I would like to take the following approach:

I wait for Cleanup tests #16 to be merged
Once Cleanup tests #16 is merged I will raise the separate PR with the *ignore tests and then reduce this one to the fs-extra commit

raphinesse · 2018-06-04T13:20:55Z

@brodybits I'm fine with that. I just thought we might accelerate the overall process if we make this PR reviewable ASAP.

brody4hire · 2018-06-15T17:49:39Z

Linking this change in progress to Apache CB-14140 (https://issues.apache.org/jira/browse/CB-14140).

brody4hire · 2018-06-15T17:52:52Z

P.S. I would like to see the actual shelljs --> fs-extra commits marked for CB-14140 here and in the other components.

brody4hire mentioned this pull request May 29, 2018

[CB-12397] .gitignore instead of unwanted .npmignore after cordova create (7.0.1) apache/cordova-discuss#69

Closed

dpogue reviewed May 29, 2018

View reviewed changes

brody4hire force-pushed the introduce-fs-extra-usage branch from e67ed99 to b5ddb10 Compare May 29, 2018 18:46

brody4hire force-pushed the introduce-fs-extra-usage branch from 4cb86ca to faa87b3 Compare May 29, 2018 19:44

brody4hire force-pushed the introduce-fs-extra-usage branch from faa87b3 to c11756a Compare May 29, 2018 20:04

brody4hire closed this May 29, 2018

brody4hire reopened this May 29, 2018

brody4hire force-pushed the introduce-fs-extra-usage branch from c11756a to f89950f Compare May 29, 2018 23:58

brody4hire force-pushed the introduce-fs-extra-usage branch from f89950f to 538a345 Compare May 30, 2018 00:40

brody4hire changed the title ~~Introduce fs-extra usage~~ fs-extra instead of shelljs May 30, 2018

brody4hire mentioned this pull request May 30, 2018

*ignore FIX WIP raphinesse/cordova-create#1

Closed

brody4hire force-pushed the introduce-fs-extra-usage branch 2 times, most recently from 5053bf5 to 92a56ed Compare May 30, 2018 19:19

brody4hire changed the title ~~fs-extra instead of shelljs~~ fs-extra instead of shelljs, with additional testing May 30, 2018

raphinesse approved these changes May 30, 2018

View reviewed changes

raphinesse and others added 2 commits May 30, 2018 17:03

spec fix error messages for toExist matcher

1aacab5

spec check more artifacts, with some refactoring

f4b752d

(some redundant test code removed) Co-authored-by: Christopher J. Brody <[email protected]> Co-authored-by: Raphael von der Grün <[email protected]>

brody4hire force-pushed the introduce-fs-extra-usage branch from 92a56ed to bbe4af9 Compare May 30, 2018 21:04

use fs-extra instead of shelljs

754076d

Co-authored-by: Christopher J. Brody <[email protected]> Co-authored-by: Raphael von der Grün <[email protected]>

brody4hire force-pushed the introduce-fs-extra-usage branch from bbe4af9 to 754076d Compare May 30, 2018 21:09

raphinesse approved these changes May 30, 2018

View reviewed changes

brody4hire mentioned this pull request May 30, 2018

CB-12397 fix .gitignore for plugins & platforms (cordova-create part) #8

Closed

3 tasks

raphinesse mentioned this pull request Jun 3, 2018

Cleanup tests #16

Merged

raphinesse changed the title ~~fs-extra instead of shelljs, with additional testing~~ WIP fs-extra instead of shelljs Jun 3, 2018

brody4hire changed the title ~~WIP fs-extra instead of shelljs~~ CB-14140 WIP fs-extra instead of shelljs Jun 15, 2018

raphinesse mentioned this pull request Jun 17, 2018

CB-14140 Use fs-extra instead of shelljs #19

Merged

raphinesse closed this in #19 Jun 17, 2018

brody4hire mentioned this pull request Nov 23, 2018

Create .gitignore with useful content apache/cordova-app-hello-world#30

Closed

brody4hire deleted the introduce-fs-extra-usage branch July 15, 2019 20:29

CB-14140 WIP fs-extra instead of shelljs #14

CB-14140 WIP fs-extra instead of shelljs #14

Conversation

brody4hire commented May 29, 2018

Platforms affected

What does this PR do?

What testing has been done on this change?

Checklist

FUTURE TODO

dpogue left a comment

Choose a reason for hiding this comment

dpogue May 29, 2018

Choose a reason for hiding this comment

brody4hire May 29, 2018

Choose a reason for hiding this comment

brody4hire May 29, 2018

Choose a reason for hiding this comment

brody4hire commented May 29, 2018

brody4hire commented May 29, 2018

brody4hire commented May 29, 2018

raphinesse commented May 29, 2018

brody4hire commented May 29, 2018 • edited Loading

raphinesse commented May 29, 2018

brody4hire commented May 29, 2018

raphinesse commented May 29, 2018 • edited Loading

brody4hire commented May 29, 2018

dpogue commented May 29, 2018

dpogue commented May 29, 2018

raphinesse commented May 29, 2018 • edited Loading

brody4hire commented May 30, 2018

raphinesse commented May 30, 2018 • edited Loading

brody4hire commented May 30, 2018 • edited Loading

brody4hire commented May 30, 2018 • edited Loading

raphinesse left a comment • edited Loading

Choose a reason for hiding this comment

raphinesse May 30, 2018

Choose a reason for hiding this comment

brody4hire May 30, 2018

Choose a reason for hiding this comment

raphinesse May 30, 2018 • edited Loading

Choose a reason for hiding this comment

raphinesse May 30, 2018

Choose a reason for hiding this comment

brody4hire May 30, 2018

Choose a reason for hiding this comment

brody4hire commented May 30, 2018

raphinesse commented May 30, 2018

brody4hire commented May 30, 2018 • edited Loading

raphinesse left a comment • edited Loading

Choose a reason for hiding this comment

brody4hire commented May 31, 2018

raphinesse commented May 31, 2018

raphinesse commented Jun 3, 2018

raphinesse commented Jun 3, 2018

raphinesse commented Jun 4, 2018

brody4hire commented Jun 4, 2018 via email

raphinesse commented Jun 4, 2018

brody4hire commented Jun 4, 2018

raphinesse commented Jun 4, 2018

brody4hire commented Jun 15, 2018

brody4hire commented Jun 15, 2018

brody4hire commented May 29, 2018 •

edited

Loading

raphinesse commented May 29, 2018 •

edited

Loading

raphinesse commented May 29, 2018 •

edited

Loading

raphinesse commented May 30, 2018 •

edited

Loading

brody4hire commented May 30, 2018 •

edited

Loading

brody4hire commented May 30, 2018 •

edited

Loading

raphinesse left a comment •

edited

Loading

raphinesse May 30, 2018 •

edited

Loading

brody4hire commented May 30, 2018 •

edited

Loading

raphinesse left a comment •

edited

Loading