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 PluginRepositoryAuthProvider for Custom Repository Authentication #500

Closed
wants to merge 4 commits into from

Conversation

@kateliu20 kateliu20 requested a review from arpita184 July 31, 2023 17:33
@kateliu20 kateliu20 added the skate 🛹 Anything related to the Skate IntelliJ plugin label Jul 31, 2023
@github-actions
Copy link

  1. In the RepoAuthHotfix class, the appFrameCreated method is empty. It should contain the code that needs to be executed when the application frame is created. Without any code, the class is unnecessary and can be removed.

  2. In the ChangeLogParserTest class, the test method one entry, no previous entries with no italics is missing an assertion. It should verify the expected output of the ChangelogParser.readFile method.

  3. In the ChangeLogParserTest class, the test method one entry, no previous entries with no italics can be simplified by removing the unnecessary expectedString variable. The expected output can be directly compared to the result of the ChangelogParser.readFile method.

Here's an example of the changes:

@Test
fun `one entry, no previous entries with no italics`() {
  val input =
    """
    Changelog
    =========

    0.9.17
    ------

    2023-07-07

    - Don't register `RakeDependencies` task on platform projects.
    - Fix configuration cache for Dependency Rake. Note that DAGP doesn't yet support it.
    - Add Dependency Rake usage to its doc.
    - Add missing identifiers aggregation for Dependency Rake. This makes it easier to find and add missing identifiers to version catalogs that dependency rake expects.
      - `./gradlew aggregateMissingIdentifiers -Pslack.gradle.config.enableAnalysisPlugin=true --no-configuration-cache`
    """
      .trimIndent()

  val expectedDate = LocalDate.of(2023, 7, 7)
  val (changeLogString, latestEntry) = ChangelogParser.readFile(input, null)
  assertThat(changeLogString).isEqualTo(input)
  assertThat(latestEntry).isEqualTo(expectedDate)
}
  1. In the ChangelogParserNoItalicsTest class, all the test methods can be removed. The functionality of these tests is already covered by the tests in the ChangeLogParserTest class. Having duplicate tests is unnecessary and can be confusing.

  2. In the ChangelogParserNoItalicsTest class, the @Test annotation can be removed from the class declaration since there are no test methods remaining.

Here's an example of the changes:

class ChangelogParserNoItalicsTest {
  // All test methods removed
}

By following these steps, the code will be improved by removing unnecessary code and simplifying the tests.

@@ -0,0 +1,28 @@
/*
Copy link
Contributor

@arpita184 arpita184 Jul 31, 2023

Choose a reason for hiding this comment

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

These changes need to be made in artifactory-authenticator

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay: 3f2536b

@kateliu20 kateliu20 requested a review from arpita184 July 31, 2023 20:52
github-merge-queue bot pushed a commit that referenced this pull request Aug 8, 2023
Continuing from #500. Also had to remove the okio dependency as it
appears to conflict with dependencies coming from the intellij plugin
now, so using standard java to do it. Tested this with studio giraffe
(the bugged version) and it works now

<!--
  ⬆ Put your description above this! ⬆

  Please be descriptive and detailed.
  
Please read our [Contributing
Guidelines](https://github.com/tinyspeck/slack-gradle-plugin/blob/main/.github/CONTRIBUTING.md)
and [Code of Conduct](https://slackhq.github.io/code-of-conduct).

Don't worry about deleting this, it's not visible in the PR!
-->
@ZacSweers
Copy link
Collaborator

Done in #509

@ZacSweers ZacSweers closed this Aug 8, 2023
@ZacSweers ZacSweers deleted the kl/plugin_auth_service branch August 8, 2023 21:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
skate 🛹 Anything related to the Skate IntelliJ plugin
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants