brendanhay / gogol

A comprehensive Google Services SDK for Haskell.
Other
282 stars 105 forks source link

Upgraded gogol-gen to work with lts-13 (ghc-8.6.3) #123

Closed korayal closed 5 years ago

korayal commented 5 years ago

This is my first meddling with haskell-src-exts, so please take caution while reviewing this PR.

What I did was mostly adding missing () which was introduced in data types from haskell-src-exts as the annotation parameter l, which it seems is not necessary in our case.

This probably shouldn't be merged until hindent package has a new release since the latest version has a bug that causes a termination of the gogol-gen. That bug is fixed in the upstream, hence the additional extra-deps inside gen/stack.yaml.

I don't think the requirements above should prevent me from sending this PR, since they're all about upstream issues that are fixed but not yet released.


Regarding this upgrade, I took gogol-drive as my basis of comparison, so the resulting files are nearly identical except some changes in style caused by a change in hindent since v5:

removed trailing ;

 -- | Modify your Google Apps Script scripts\' behavior
 driveScriptsScope :: Proxy '["https://www.googleapis.com/auth/drive.scripts"]
-driveScriptsScope = Proxy;
+driveScriptsScope = Proxy

deriving and FileList' moved to a new line

 -- | A list of files.
 --
 -- /See:/ 'fileList' smart constructor.
-data FileList = FileList'
+data FileList =
+  FileList'
     { _flNextPageToken    :: !(Maybe Text)
     , _flIncompleteSearch :: !(Maybe Bool)
     , _flKind             :: !Text
     , _flFiles            :: !(Maybe [File])
-    } deriving (Eq,Show,Data,Typeable,Generic)
+    }
+  deriving (Eq, Show, Data, Typeable, Generic)
+

if a definition fits in a single line without breaking column limits, it is kept as so.

 fileAppProperties pFapAddtional_ =
-    FileAppProperties'
-    { _fapAddtional = _Coerce # pFapAddtional_
-    }
+  FileAppProperties' {_fapAddtional = _Coerce # pFapAddtional_}
+

After making sure that generated package is identical to the one we generated before in v0.4.0, I went ahead and tried compiling the result, which was successful. Then I've generated all of the gogol-* packages using the latest models from Google's repo, and they too were successfully compiled.

I have not added the resulting packages, since it causes a huge noise in the diff of the PR. So, if this PR gets merged successfully, I'll go ahead and prepare the gogol-* packages for the next release.

There is one caveat though, on the last version of models, there is a new model called docs, which this generator fails to create it's respecting gogol-docs library. It'll probably require a more detailed work, and I don't think it'll also work for the older generator.

Since it's nearly impossible to check all of the diffs, I expect @brendanhay's input to look for edge cases.

note: I'm not very proud of using unsafe methods in gen/src/Gen/Types/Help.hs to prevent introducing monads to the source.

korayal commented 5 years ago

I've found a better way to check out the changes occurred in gogol-* packages' source:

After regenerating all of the package running make on the gen folder, run this git diff command (which uses word-diff but also ignores whitespaces):

git diff --unified=0 --word-diff-regex="[^[:space:]]" | grep -E "\{\+|\[-|+++---"

I dumped this result to a file and started analyzing, and it looks like there are no breaking changes whatsoever:

just some tables within comments (which is a lot more readable now):

--- a/gogol-composer/gen/Network/Google/Resource/Composer/Projects/Locations/Environments/Patch.hs
+++ b/gogol-composer/gen/Network/Google/Resource/Composer/Projects/Locations/Environments/Patch.hs
@@ -152,17 +154,89 @@ plepUploadProtocol
 -- be the following: { \"config\":{ \"softwareConfig\":{ \"pypiPackages\":{
 -- \"botocore\":\"==1.7.14\" } } } } **Note:** Only the following fields
 -- can be updated:
--- >   ----------------------------------------------------------- -----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
--- >   __Mask__                                                    __Purpose__
--- >   config.softwareConfig.pypiPackages                          Replace all custom custom PyPI packages. If a replacement package map is not included in \`environment\`, all custom PyPI packages are cleared. It is an error to provide both this mask and a mask specifying an individual package.
--- >   config.softwareConfig.pypiPackages.packagename              Update the custom PyPI package packagename, preserving other packages. To delete the package, include it in \`updateMask\`, and omit the mapping for it in \`environment.config.softwareConfig.pypiPackages\`. It is an error to provide both a mask of this form and the \"config.softwareConfig.pypiPackages\" mask.
--- >   labels                                                      Replace all environment labels. If a replacement labels map is not included in \`environment\`, all labels are cleared. It is an error to provide both this mask and a mask specifying one or more individual labels.
--- >   labels.labelName                                            Set the label named labelName, while preserving other labels. To delete the label, include it in \`updateMask\` and omit its mapping in \`environment.labels\`. It is an error to provide both a mask of this form and the \"labels\" mask.
--- >   config.nodeCount                                            Horizontally scale the number of nodes in the environment. An integer greater than or equal to 3 must be provided in the \`config.nodeCount\` field.
--- >   config.softwareConfig.airflowConfigOverrides                Replace all Apache Airflow config overrides. If a replacement config overrides map is not included in \`environment\`, all config overrides are cleared. It is an error to provide both this mask and a mask specifying one or more individual config overrides.
--- >   config.softwareConfig.airflowConfigOverrides.section-name   Override the Apache Airflow config property name in the section named section, preserving other properties. To delete the property override, include it in \`updateMask\` and omit its mapping in \`environment.config.softwareConfig.airflowConfigOverrides\`. It is an error to provide both a mask of this form and the \"config.softwareConfig.airflowConfigOverrides\" mask.
--- >   config.softwareConfig.envVariables                          Replace all environment variables. If a replacement environment variable map is not included in \`environment\`, all custom environment variables are cleared. It is an error to provide both this mask and a mask specifying one or more individual environment variables.
--- >   ----------------------------------------------------------- -----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
+-- > +-----------------------------------+-----------------------------------+
+-- > | __Mask__                          | __Purpose__                       |
+-- > +-----------------------------------+-----------------------------------+
+-- > | config.softwareConfig.pypiPackage | Replace all custom custom PyPI    |
+-- > | s                                 | packages. If a replacement        |
+-- > |                                   | package map is not included in    |
+-- > |                                   | \`environment\`, all custom PyPI  |
+-- > |                                   | packages are cleared. It is an    |
+-- > |                                   | error to provide both this mask   |
+-- > |                                   | and a mask specifying an          |
+-- > |                                   | individual package.               |
+-- > +-----------------------------------+-----------------------------------+
+-- > | config.softwareConfig.pypiPackage | Update the custom PyPI package    |
+-- > | s.packagename                     | packagename, preserving other     |
+-- > |                                   | packages. To delete the package,  |
+-- > |                                   | include it in \`updateMask\`, and |
+-- > |                                   | omit the mapping for it in        |
+-- > |                                   | \`environment.config.softwareConf |
+-- > |                                   | ig.pypiPackages\`.                |
+-- > |                                   | It is an error to provide both a  |
+-- > |                                   | mask of this form and the         |
+-- > |                                   | \"config.softwareConfig.pypiPacka |
+-- > |                                   | ges\"                             |
+-- > |                                   | mask.                             |
+-- > +-----------------------------------+-----------------------------------+
+-- > | labels                            | Replace all environment labels.   |
+-- > |                                   | If a replacement labels map is    |
+-- > |                                   | not included in \`environment\`,  |
+-- > |                                   | all labels are cleared. It is an  |
+-- > |                                   | error to provide both this mask   |
+-- > |                                   | and a mask specifying one or more |
+-- > |                                   | individual labels.                |
+-- > +-----------------------------------+-----------------------------------+
+-- > | labels.labelName                  | Set the label named labelName,    |
+-- > |                                   | while preserving other labels. To |
+-- > |                                   | delete the label, include it in   |
+-- > |                                   | \`updateMask\` and omit its       |
+-- > |                                   | mapping in                        |
+-- > |                                   | \`environment.labels\`. It is an  |
+-- > |                                   | error to provide both a mask of   |
+-- > |                                   | this form and the \"labels\"      |
+-- > |                                   | mask.                             |
+-- > +-----------------------------------+-----------------------------------+
+-- > | config.nodeCount                  | Horizontally scale the number of  |
+-- > |                                   | nodes in the environment. An      |
+-- > |                                   | integer greater than or equal to  |
+-- > |                                   | 3 must be provided in the         |
+-- > |                                   | \`config.nodeCount\` field.       |
+-- > +-----------------------------------+-----------------------------------+
+-- > | config.softwareConfig.airflowConf | Replace all Apache Airflow config |
+-- > | igOverrides                       | overrides. If a replacement       |
+-- > |                                   | config overrides map is not       |
+-- > |                                   | included in \`environment\`, all  |
+-- > |                                   | config overrides are cleared. It  |
+-- > |                                   | is an error to provide both this  |
+-- > |                                   | mask and a mask specifying one or |
+-- > |                                   | more individual config overrides. |
+-- > +-----------------------------------+-----------------------------------+
+-- > | config.softwareConfig.airflowConf | Override the Apache Airflow       |
+-- > | igOverrides.section-name          | config property name in the       |
+-- > |                                   | section named section, preserving |
+-- > |                                   | other properties. To delete the   |
+-- > |                                   | property override, include it in  |
+-- > |                                   | \`updateMask\` and omit its       |
+-- > |                                   | mapping in                        |
+-- > |                                   | \`environment.config.softwareConf |
+-- > |                                   | ig.airflowConfigOverrides\`.      |
+-- > |                                   | It is an error to provide both a  |
+-- > |                                   | mask of this form and the         |
+-- > |                                   | \"config.softwareConfig.airflowCo |
+-- > |                                   | nfigOverrides\"                   |
+-- > |                                   | mask.                             |
+-- > +-----------------------------------+-----------------------------------+
+-- > | config.softwareConfig.envVariable | Replace all environment           |
+-- > | s                                 | variables. If a replacement       |
+-- > |                                   | environment variable map is not   |
+-- > |                                   | included in \`environment\`, all  |
+-- > |                                   | custom environment variables are  |
+-- > |                                   | cleared. It is an error to        |
+-- > |                                   | provide both this mask and a mask |
+-- > |                                   | specifying one or more individual |
+-- > |                                   | environment variables.            |
+-- > +-----------------------------------+-----------------------------------+

and some improvements:

--- a/gogol-firebase-dynamiclinks/gen/Network/Google/FirebaseDynamicLinks/Types/Product.hs
+++ b/gogol-firebase-dynamiclinks/gen/Network/Google/FirebaseDynamicLinks/Types/Product.hs
@@ -1148,8 +1160,7 @@ gipiarDevice :: Lens' GetIosPostInstallAttributionRequest (Maybe DeviceInfo)
 gipiarDevice
   = lens _gipiarDevice (\ s a -> s{_gipiarDevice = a})

--- | Google SDK version. Version takes the form
--- \"/m//a//j//o//r/.minor.$patch\"
+-- | Google SDK version. Version takes the form \"$major.$minor.$patch\"
 gipiarSdkVersion :: Lens' GetIosPostInstallAttributionRequest (Maybe Text)
 gipiarSdkVersion
   = lens _gipiarSdkVersion
MichaelXavier commented 5 years ago

This seems like a good idea to rip off the bandage now so its easier to contribute to and work on gogol-gen.