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

Extract exclude rules to separate files, abstract away from Gradle. #51

Merged
merged 3 commits into from
Jan 5, 2017

Conversation

artem-zinnatullin
Copy link
Contributor

Closes #19.

This allows mainframer.sh to support basically any build system, see samples/gradle for gradle example.

@@ -40,32 +42,39 @@ if [ -z "$BUILD_COMMAND" ]; then
exit 1
fi

pushd "$PROJECT_DIR"
# Read local exclude rules.
LOCAL_EXCLUDE_FILE=".mainframerignorelocal"
Copy link
Contributor

Choose a reason for hiding this comment

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

What if we name this files .mfignore.local and .mfignore.remote?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm thinking about separate folder for mainframer configs where we could place .ignorelocal, .ignoreremote and #50 .config

@@ -15,6 +15,27 @@ function property {
grep "^${1}=" $PROJECT_DIR/local.properties | cut -d'=' -f2
}

function readExcludesFile {
Copy link
Contributor

Choose a reason for hiding this comment

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

What’s wrong with rsync --exclude-from?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nothing, will try

@@ -40,32 +61,21 @@ if [ -z "$BUILD_COMMAND" ]; then
exit 1
fi

pushd "$PROJECT_DIR"
# Read exclude rules.
LOCAL_EXCLUDE="$(readExcludesFile .mainframerignorelocal)"
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure about that. What about .mainframerlocalignore?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See #54. I'd vote for current names at the moment and then if we decide to move configs into a separate folder -> rename this files, names are not the point of this PR.

--exclude='**/build' \
--exclude='**/local.properties' \
--rsh "ssh" ./ "$REMOTE_BUILD_MACHINE:~/$PROJECT_DIR_NAME"
eval "rsync --archive --delete --compress-level=$LOCAL_COMPRESS_LEVEL $LOCAL_EXCLUDE --rsh ssh ./ $REMOTE_BUILD_MACHINE:~/$PROJECT_DIR_NAME"
Copy link
Contributor

@arturdryomov arturdryomov Jan 4, 2017

Choose a reason for hiding this comment

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

At this point we can make this a function with four arguments. Not sure if it’s worth it though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No need for now

@@ -0,0 +1,5 @@
.gradle
Copy link
Contributor

Choose a reason for hiding this comment

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

Samples don’t entirely repeat what we have at this point.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yup, it's basic gradle sample, not gradle-android

Copy link
Contributor

Choose a reason for hiding this comment

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

gradle-android will make more sense since our userbase mostly consists of Android developers (for now).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will do in a separate pr.

--exclude='artifacts' \
--exclude='captures' \
--exclude='**/build' \
--exclude='**/local.properties' \
Copy link
Contributor

Choose a reason for hiding this comment

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

Remote and local excludes look very similar. .mainframerignorecommon?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, that's totally up to each project members

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Though, ok, this looks like a valid feature, let's think about it and add later if we'll agree on it

@artem-zinnatullin
Copy link
Contributor Author

Switched to rsync builtin --exclude-from, PTAL!

I'd vote for merging as is and adding other things in separate PRs.

@artem-zinnatullin artem-zinnatullin merged commit 5499c16 into master Jan 5, 2017
@artem-zinnatullin artem-zinnatullin deleted the tech/az/exclude-files branch January 5, 2017 10:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants