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

CB-13927 - Modified xcodeProjDir to filter out files/folders that contain "._" #355

Merged
merged 2 commits into from
Jan 9, 2019

Conversation

asoap
Copy link
Contributor

@asoap asoap commented Jan 31, 2018

I ran into an issue trying to add an ios plugin using a virtual machine. The code would return an array for the xcodeProjDir and then of course use the wrong one. It would use the one starting with "._ProjectName" which I believe is an MacOS hidden folder. I attempted to install the plugin on OsX but because it's a virtual machine it still had those hidden folders. I've made it so that it will filter those out while looking for the proper xCode project folder.

Platforms affected

Windows (host) / OSx (guest) virtual machines

What does this PR do?

Fixes a folder issue. Before it would try to use an incorrect folder for the xCode project.

What testing has been done on this change?

I made the change while trying to install a patch. It should work the same as before as this code is only executed if there is more than one folder returned from searching the file system.

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.

I ran into an issue trying to add an ios plugin using a virtual machine.  The code would return an array for the xcodeProjDir and then of course use the wrong one.  It would use the one starting with "._ProjectName" which I believe is an MacOS hidden folder.  I attempted to install the plugin on OsX but because it's a virtual machine it still had those hidden folders.   I've made it so that it will filter those out while looking for the proper xCode project folder.
@asoap
Copy link
Contributor Author

asoap commented Jan 31, 2018

I'm new to this, and I see my code failed the test. Is this the only thing it failed on?

C:\projects\cordova-ios\bin\templates\scripts\cordova\Api.js
67:1 error Trailing spaces not allowed no-trailing-spaces
70:1 error Expected indentation of 12 spaces but found 10 indent
71:1 error Expected indentation of 16 spaces but found 12 indent
71:50 error A space is required after ',' comma-spacing
72:1 error Expected indentation of 20 spaces but found 14 indent
72:42 error A space is required after ',' comma-spacing
73:1 error Expected indentation of 16 spaces but found 12 indent
74:1 error Expected indentation of 12 spaces but found 10 indent
77:1 error Trailing spaces not allowed no-trailing-spaces

@surajpindoria
Copy link
Member

Hi @asoap,

Looks like those are the only errors, so fixing these should be enough to get the CI passing

@codecov-io
Copy link

Codecov Report

Merging #355 into master will decrease coverage by 0.06%.
The diff coverage is 50%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #355      +/-   ##
==========================================
- Coverage   63.45%   63.38%   -0.07%     
==========================================
  Files          14       14              
  Lines        1691     1696       +5     
  Branches      284      286       +2     
==========================================
+ Hits         1073     1075       +2     
- Misses        618      621       +3
Impacted Files Coverage Δ
bin/templates/scripts/cordova/Api.js 53.45% <50%> (-0.44%) ⬇️

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 2e51796...c8f704c. Read the comment docs.

@asoap
Copy link
Contributor Author

asoap commented Feb 1, 2018

YAY!!!!!

Look at me, I'm helping. 👯‍♂️

@shazron
Copy link
Member

shazron commented Feb 26, 2018

@asoap I would like to get this in, however two things are needed:

  1. File an issue in issues.cordova.io
  2. Prefix the title of this PR with the JIRA issue tag (i.e. CB-XXXX)

@shazron
Copy link
Member

shazron commented Feb 27, 2018

Ping @asoap . If no progress on this for this week, this will be bumped until next release.

}
}
xcodeProjDir = xcodeProjDir_array[0];

Copy link
Member

@shazron shazron Feb 28, 2018

Choose a reason for hiding this comment

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

Thanks! I think this would be more efficient and clearer, what do you think?

var xcodeProjDir_array = fs
    .readdirSync(this.root)
    .filter(function (elem) {
        // match .xcodeproj, and not start with '._'
        return elem.match(/\.xcodeproj$/i) && (elem.substring(0, 2) !== '._'); 
    })

xcodeProjDir = xcodeProjDir_array[0];      

@shazron shazron changed the title Modified xcodeProjDir to filter out files/folders that contain "._" CB-13927 - Modified xcodeProjDir to filter out files/folders that contain "._" Feb 28, 2018
@shazron
Copy link
Member

shazron commented Feb 28, 2018

Added JIRA issue, modified the title, and commented on the code.

@shazron shazron merged commit 1cf9224 into apache:master Jan 9, 2019
erisu pushed a commit to erisu/cordova-ios that referenced this pull request Jan 16, 2019
…tain "._" (apache#355)

* Modified xcodeProjDir to filter out files/folders that contain "._"

I ran into an issue trying to add an ios plugin using a virtual machine.  The code would return an array for the xcodeProjDir and then of course use the wrong one.  It would use the one starting with "._ProjectName" which I believe is an MacOS hidden folder.  I attempted to install the plugin on OsX but because it's a virtual machine it still had those hidden folders.   I've made it so that it will filter those out while looking for the proper xCode project folder.
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