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

Project support #537

Merged
merged 17 commits into from
Aug 17, 2021
Merged

Project support #537

merged 17 commits into from
Aug 17, 2021

Conversation

anthmatic
Copy link
Contributor

@anthmatic anthmatic commented Aug 16, 2021

Description and Context

Updated project support:

projects renamed to project

  • hs project init
  • hs project upload

Changes to hsproject.json:

{
  "name": "weather_app",
  "srcDir": "src"
}

NOTE: I've purposely left some TODOs that I'll address later, I'd just like to get the basics in so that we can get a solid feedback loop in.

Screenshots

TODO

Who to Notify

@anthmatic anthmatic marked this pull request as ready for review August 17, 2021 16:51
describe: 'Path to a project folder',
type: 'string',
});
// TODO:
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 was previously accounting for these to be passed as options. Not exactly sure how that should work with the new config file. Thinking of keeping these and getting some UX feedback.

exports.describe = false; //'Commands for working with projects';

exports.builder = yargs => {
addOverwriteOptions(yargs, true);
addConfigOptions(yargs, true);
addAccountOptions(yargs, true);

// TODO: deploy must be updated
yargs.command(deploy).demandCommand(1, '');
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not currently used in the flow. Will need to be adjusted eventually, but will mostly stay the same so keeping this for now.

uri: DEVELOPER_FILE_SYSTEM_API_PATH,
});
}
// TODO: paging?
Copy link
Contributor

Choose a reason for hiding this comment

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

Did we want to add paging, or is this a reminder for later?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We'll need to add paging but I'll remove the TODO for now

const { path: projectPath } = options;
const accountId = getAccountId(options);

// TODO:
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we enable this now?

Comment on lines +74 to +87
logger.log('');
logger.log('> Welcome to HubSpot Developer Projects!');
logger.log('-------------------------------------------------------------');
logger.log('Getting Started');
logger.log('1. hs project upload');
logger.log(
' Upload your project files to HubSpot. Upload action adds your files to a build.'
);
logger.log();
logger.log('2. View your changes on the preview build url');
logger.log();
logger.log('Use `hs project --help` to learn more about the command.');
logger.log('-------------------------------------------------------------');
};
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@brandenrodgers
Copy link
Contributor

Pending answers to Mike's questions this all lgtm! This is a solid first pass 👌

Copy link
Contributor

@miketalley miketalley left a comment

Choose a reason for hiding this comment

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

LGTM -- Just wanted to check if we should address the TODOs now or if they are meant to be left for later.

@anthmatic
Copy link
Contributor Author

if we should address the TODOs now or if they are meant to be left for later

These were mostly reminders for me to come back to later or enable things like tracking when we really need it but in the interest of keeping things cleaner I enabled tracking and removed most of the TODOs for now.

@anthmatic anthmatic merged commit 318f7d4 into master Aug 17, 2021
@anthmatic anthmatic deleted the add/project-support branch August 17, 2021 22:14
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.

3 participants