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

Add awscli preliminary tests and refactor python language. #46

Merged
merged 1 commit into from
Jun 28, 2017

Conversation

harshavardhana
Copy link
Member

@harshavardhana harshavardhana commented Jun 27, 2017

  • This PR only changes python, so that other languages
    can follow. Adds a new package awscli for awscli tests.

  • Adds awscli preliminary tests

Refer #44

@harshavardhana harshavardhana force-pushed the cleanup-add-aws-cli branch 3 times, most recently from cc8cdb7 to 5fce0b9 Compare June 27, 2017 23:02
@harshavardhana harshavardhana changed the title Move language into their own specific files. Add awscli preliminary tests and refactor python language. Jun 27, 2017

cleanDeps() {
# Nothing yet.
echo
Copy link
Contributor

Choose a reason for hiding this comment

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

can we remove pip while cleanup?

Copy link
Member Author

Choose a reason for hiding this comment

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

Not here we remove in install.sh

apt-get autoremove -yq
cleanDeps() {
# Nothing yet.
echo
}
Copy link
Contributor

@nitisht nitisht Jun 27, 2017

Choose a reason for hiding this comment

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

can remove cleanDeps() if not required here.

Copy link
Contributor

@nitisht nitisht left a comment

Choose a reason for hiding this comment

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

other than one comment, LGTM

@nitisht nitisht requested a review from poornas June 28, 2017 00:42
@nitisht
Copy link
Contributor

nitisht commented Jun 28, 2017

Can you pls take a look @poornas

run.sh Outdated

do
for i in ${root_dir}/${test_dir}/*;

Copy link
Contributor

Choose a reason for hiding this comment

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

whitespace unnecessary here

Copy link
Member Author

Choose a reason for hiding this comment

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

In-fact there is a huge formatting issue on the entire run.sh, i just ran linting all the shell scripts have this problem. I will avoid fixing these in this PR to avoid git history problems.

We should use a better editors here to avoid these type of problems @nitisht @poornas

run.sh Outdated
if [ -d ${i} ]; then

# Will not run if no directories are available
sdk="$(basename $i)"
echo "Running $sdk tests ..."

Copy link
Contributor

Choose a reason for hiding this comment

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

no need for white space

Copy link
Member Author

Choose a reason for hiding this comment

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

Same as above.

@@ -1,4 +1,4 @@
#!/usr/bin/env bash
#!/bin/bash
#
# Minio Cloud Storage, (C) 2017 Minio, Inc.
Copy link
Contributor

@poornas poornas Jun 28, 2017

Choose a reason for hiding this comment

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

s/Minio/Mint

Copy link
Contributor

@poornas poornas left a comment

Choose a reason for hiding this comment

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

looks good - couple of typos

@harshavardhana
Copy link
Member Author

looks good - couple of typos

Fixed @poornas

- This PR only changes python, so that other languages
  can follow. Adds a new package `awscli` for `awscli` tests.

- Adds awscli preliminary tests

Refer minio#44
@nitisht nitisht merged commit 0a14096 into minio:master Jun 28, 2017
@harshavardhana harshavardhana deleted the cleanup-add-aws-cli branch June 28, 2017 02:28
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.

None yet

3 participants