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

Http mock update #79

Merged
merged 8 commits into from
Oct 28, 2018
Merged

Http mock update #79

merged 8 commits into from
Oct 28, 2018

Conversation

pragnagopa
Copy link
Member

PR Azure/azure-functions-java-library#60 udpated HttpResponseMessage API.
This PR is updating the archetype to consume API changes

@pragnagopa
Copy link
Member Author

Note: PR is not merged into library repo yet due to github issues.

@pragnagopa
Copy link
Member Author

Library changes merged in and published to sonatype repo
https://oss.sonatype.org/content/repositories/commicrosoftazure-2669

@pragnagopa
Copy link
Member Author

Ping!

@jdneo jdneo changed the base branch from master to develop October 24, 2018 01:07
<functionAppName>${appName}</functionAppName>
<functionAppRegion>${appRegion}</functionAppRegion>
<stagingDirectory>${project.build.directory}/azure-functions/${functionAppName}</stagingDirectory>
<functionResourceGroup>${resourceGroup}</functionResourceGroup>
</properties>

<repositories>
Copy link
Member

Choose a reason for hiding this comment

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

Does the <repositories> attribute only exist for test purpose?

@pragnagopa
Copy link
Member Author

Added Snapshot repository. Please publish snapshot version of the archetype and maven plugin with the latest changes and update the versions as well

@pragnagopa
Copy link
Member Author

I have tested project builds with latest bits from archetype, plugin and azure-functions-java-library

@@ -1,4 +1,4 @@
package $package;
package com.microsoft;
Copy link
Member

Choose a reason for hiding this comment

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

revert to $package

Copy link
Member

@jdneo jdneo left a comment

Choose a reason for hiding this comment

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

The newly added files are using tab as indention. Please change it to spaces with size 4

@pragnagopa
Copy link
Member Author

Addressed CR. Please review

@@ -14,13 +14,27 @@
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
<maven.compiler.source>1.8</maven.compiler.source>
<maven.compiler.target>1.8</maven.compiler.target>
<azure.functions.maven.plugin.version>1.0.0-beta-7</azure.functions.maven.plugin.version>
<azure.functions.java.library.version>1.0.0-beta-5</azure.functions.java.library.version>
<azure.functions.maven.plugin.version>1.0.0-beta-7</azure.functions.maven.plugin.version>
Copy link
Member

Choose a reason for hiding this comment

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

tailing spaces are not needed.

Copy link
Member Author

Choose a reason for hiding this comment

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

Which IDE do you recommend? Is there a settings file you can share?

Copy link
Member

Choose a reason for hiding this comment

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

I'm using VS Code 😃

You can install an extension called: XML Tools. Set the indention to spaces with size 4. And then trigger format command.

<functionAppName>${appName}</functionAppName>
<functionAppRegion>${appRegion}</functionAppRegion>
<stagingDirectory>${project.build.directory}/azure-functions/${functionAppName}</stagingDirectory>
<functionResourceGroup>${resourceGroup}</functionResourceGroup>
</properties>

<repositories>
Copy link
Member

@jdneo jdneo Oct 26, 2018

Choose a reason for hiding this comment

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

tab -> spaces in file

<enabled>true</enabled>
</snapshots>
</repository>
</repositories>
Copy link
Member

Choose a reason for hiding this comment

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

A question not a review comment here:
Should we leave the field as it is when we releasing to Maven Central? Or we need to remove it?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes. You can leave it in here. For future releases SNAPSHOT versions can be referenced from 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.

Thanks. Used VSCode!

@pragnagopa pragnagopa mentioned this pull request Oct 26, 2018
@Flanker32 Flanker32 merged commit 02c96f0 into microsoft:develop Oct 28, 2018
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.

3 participants