Skip to content

Start livesync watcher before preparing the project #3576

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

Merged
merged 2 commits into from
May 16, 2018
Merged

Conversation

Fatme
Copy link
Contributor

@Fatme Fatme commented May 11, 2018

PR Checklist

What is the current behavior?

Currently the project is prepared and after that a livesync watcher is started.

What is the new behavior?

LiveSync watcher is started and after that the project is prepared.

Implemented #3404

@Fatme Fatme force-pushed the fatme/watch branch 2 times, most recently from ccaafd8 to caeba7f Compare May 11, 2018 13:49
@Fatme
Copy link
Contributor Author

Fatme commented May 11, 2018

run ci

@Fatme
Copy link
Contributor Author

Fatme commented May 14, 2018

run ci

Copy link
Contributor

@rosen-vladimirov rosen-vladimirov left a comment

Choose a reason for hiding this comment

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

Great job!
Some minor comments.

const result: IStringDictionary = {};

const action = async (file: string) => {
if (this.$fs.getFsStats(file).isFile()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe you should try-catch this call, in case someone deletes the file while we are executing a previous chunk

@@ -58,9 +59,12 @@ export class ProjectChangesService implements IProjectChangesService {
public async checkForChanges(platform: string, projectData: IProjectData, projectChangesOptions: IProjectChangesOptions): Promise<IProjectChangesInfo> {
const platformData = this.$platformsData.getPlatformData(platform, projectData);
this._changesInfo = new ProjectChangesInfo();
if (!this.ensurePrepareInfo(platform, projectData, projectChangesOptions)) {
const isPrepareInfoEnsured = await this.ensurePrepareInfo(platform, projectData, projectChangesOptions);
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe the variable should be called isNewPrepareInfo

@@ -300,5 +305,20 @@ export class ProjectChangesService implements IProjectChangesService {
}
return false;
}

private getAppFiles(appDirectoryPath: string): string[] {
return this.$fs.enumerateFilesInDirectorySync(appDirectoryPath, (filePath: string, stat: IFsStats) => filePath.indexOf(APP_RESOURCES_FOLDER_NAME) === -1);
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe it is better to ensure the path.basename(filePath) !== appresources name
You should not use the APP_RESOURCES_FOLDER_NAME constant here - you should read the value from projectData as by using .nsconfig the user might change directory's name.

Copy link
Contributor

@rosen-vladimirov rosen-vladimirov left a comment

Choose a reason for hiding this comment

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

After green build

@Fatme Fatme merged commit 932fd75 into master May 16, 2018
@Fatme Fatme deleted the fatme/watch branch May 16, 2018 08:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants