This repository has been archived by the owner on Nov 10, 2023. It is now read-only.
Quick Start: Make More Readable Code Listings #1
Assignees
Labels
Comments
|
Thans for the report @commonsguy. We're going to leave this as is for now. Further down the road, we might create a quickstart project that you can clone. |
|
@rowillia Even with a |
ghost
assigned 
|
I don't really see anything awful on the manual quickstart page. @bolinfest, what did you have in mind in terms of improving it? |
sdwilsh
pushed a commit
that referenced
this issue
Feb 8, 2015
Summary: This diff call different buildRules to generate the main aar components 1. new BuildRule `AssembleDirectories` to assemble all resource and assets into `res/` and `assets` 2. AndroidResource which depends on #1 to generate `R.txt` 3. AndroidLibrary to generate `classes.jar` Note that if `srcs` and `res` are missing, there is no JAR is generated, will handle this in another diff. Test Plan: new unittest to test `AssembleDirectories` and then `arc unit`
|
This feels "good enough" for now. We'll probably be heavily revamping our docs in the next two months, so this will get a nice pass over it too, but I'm closing this for now. |
ghost
pushed a commit
that referenced
this issue
May 13, 2016
Summary: Network I/O is usually blocking some flow waiting to get an answer. Run those requests on a different thread pool for better CPU utilization. Diff applies this concept to CachingBuildEngine, fetching artifacts over the network. Test Plan: * Small tweaks to unit tests * CI Reviewed By: Coneko fbshipit-source-id: c5d71b4
ghost
pushed a commit
that referenced
this issue
May 19, 2016
Summary: The first step to make DefaultAndroidDirectoryResolver not use Suppliers and not throw when something is misconfigured but not used. Test Plan: * Unit tests tweaks * CI Reviewed By: Coneko fbshipit-source-id: 7f67d3f
ghost
pushed a commit
that referenced
this issue
May 19, 2016
Summary: * Introduction of new tests that check the new code. * Found and fixed a case where the SDK path points to a non-directory file and it should throw the appropriate message. Test Plan: CI Reviewed By: Coneko fbshipit-source-id: 2016a15
ghost
pushed a commit
that referenced
this issue
May 26, 2016
Summary: This diff makes part of `ManifestRuleKeys` fetching happen on the network executor. Another diff will introduce the same asynchronous flow for line `context.getArtifactCache().fetch(manifestKey.get().getFirst(), tempFile);`. Test Plan: CI Reviewed By: Coneko fbshipit-source-id: a4feec7
facebook-github-bot
pushed a commit
that referenced
this issue
Nov 5, 2016
Summary:
Some object files generated by the OCaml compiler (`.cmi` files, for instance)
may contain references to the md5 digest of other object files in their
dependency table. This can interfere with Buck's current OCaml compilation model
in interesting ways:
1. If the OCaml compiler references stale output for a given target (via, say
`.cmo` files left in a build folder after some files were moved to a
different target), the hashes expressed for the same interface in different
OCaml binaries may not agree. This leads to the dreaded `make inconsistent
assumptions over interface Foo_bar` errors we all know and love.
2. The first problem might be resolved by cleaning your output folder but,
even if you clean, some targets may have still compiled correctly against
the out-of-date build output during the previous build. Those targets would
have been written to the cache in a bogus form, leading to new and
interesting errors being raised after they're pulled from the cache on the
next build.
Currently, the only "solution" to this problem is to clean your targets and nuke
the cache between builds. (We recently disabled build caching for OCaml targets
to mitigate these problems.)
These changes take a step towards improving the situation by adding a new rule
for OCaml targets, `OCamlClean`. This rule is added as a dependency for
fine-grained `MLCompile` targets, and ensures that the relevant OCaml `bc` and
`opt` gen folders are properly cleaned for specific build targets. This will
prevent problem #1 from occuring and, by extension, problem #2 as well. IIUC,
this should allow us to reenable caching for OCaml build rules.
Test Plan: CI
Reviewed By: k21
fbshipit-source-id: 3712224
runningcode
pushed a commit
to runningcode/buck
that referenced
this issue
Apr 26, 2017
* commit '2b64271f46b16b405aec59dfae6b1aaa4b5e58dc': Fix Intellij project set up for buck.
facebook-github-bot
pushed a commit
that referenced
this issue
Aug 21, 2017
Summary:
When Xcode compiles static Swift libs, it will include linker commands (`LC_LINKER_OPTION`) that will be carried over for the final binary to link to the appropriate Swift overlays and libs. This means that the final binary must be able to locate the Swift libs in the library search path. If an Xcode target includes Swift, Xcode will automatically append the Swift lib folder when invoking the linker. For example, it will insert the following path:
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/lib/swift/macosx
which can be dynamically specified as:
$DT_TOOLCHAIN_DIR/usr/lib/swift/$PLATFORM_NAME
Unfortunately, this will not happen if we have a plain `apple_binary` that has Swift deps. So we're manually doing exactly what Xcode does to make sure binaries link successfully if they use Swift directly or transitively.
To extract `LC_LINKER_OPTION` from a static library:
otool -l libFoo.a
You will be able to see output like:
Load command 17
cmd LC_LINKER_OPTION
cmdsize 32
count 1
string #1 -lswiftCoreGraphics
Test Plan: Automated tests part of diff + manual Buck setup test
Reviewed By: VTopoliuk
fbshipit-source-id: f76171b
romanoid
added a commit
to romanoid/buck
that referenced
this issue
May 2, 2018
This solution is facebook#1 option for issue mentioned in facebook#1830
facebook-github-bot
pushed a commit
that referenced
this issue
Jul 31, 2018
Reviewed By: styurin fbshipit-source-id: 8fcae2dd97
facebook-github-bot
pushed a commit
that referenced
this issue
Oct 3, 2019
Summary: EDIT Adds a new class, `RemoteExecutionStateRenderer` to essentially do what `BuildThreadStateRenderer` does for non-RE builds. RE can't directly re-use `BuildThreadStateRenderer` since events can't be tracked by thread. Also considered putting the rendering code directly in `SuperConsoleEventBusListener` rather than making `RemoteExecutionStateRenderer`, as using `RemoteExecutionStateRenderer` adds some unnecessary complexity, such as the ID generating logic enforced by the interface. See v9 for this version (note: code wasn't cleaned up). IMO the code looked kind of weird though since local builds delegate the line creating logic to `BuildThreadStateRenderer` from `SuperConsoleEventBusListener`, while remote builds were handled directly by `SuperConsoleEventBusListener`. Another option is to make `RemoteExecutionStateRenderer` not implement `MultiStateRenderer` to reduce some complexity in `RemoteExecutionStateRenderer`, and make `SuperConsoleEventBusListener` know directly about `RemoteExecutionStateRenderer` rather than only about `MultiStateRenderer`. This approach breaks encapsulation :( --------------------- Initial thought was to have `RemoteExecutionConsoleLineProvider` add the additional lines to be printed so that all the RE console printing logic is encapsulated in this class. However, this class doesn't readily have access to the state associated with each target, so it would have to either: 1. Maintain state about the targets, maybe through a new class like `RemoteExecutionTargetsProvider` that is handed to `RemoteExecutionConsoleLineProvider` in the constructor (i.e. like how `RemoteExecutionStatsProvider` is handed to `RemoteExecutionConsoleLineProvider`. In `createConsoleLinesAtTime`, `RemoteExecutionConsoleLineProvider` grabs each target's state from `RemoteExecutionTargetsProvider` 2. Similar to #1, but instead of passing a target provider to `RemoteExecutionConsoleLineProvider`, instead `RemoteExecutionConsoleLineProvider` can subscribe to changes in the target state, and maintain state directly (through a `ConcurrentHashMap`?) in `RemoteExecutionConsoleLineProvider`. In `createConsoleLinesAtTime`, `RemoteExecutionConsoleLineProvider` grabs each target's state from the map and clears the map. Not sure what the threading model is like here (i.e. what thread the subscribe methods are called on), so may need synchronization here. 3. [THIS DIFF] The simplest approach, with the disadvantage of not being as well encapsulated: `SuperConsoleEventBusListener` subscribes to changes in RE events and prints them accordingly. D17244231 is similar to options 1 and 2 but adds the state tracking map to `RemoteExecutionStatsProvider` Reviewed By: rajyengi shipit-source-id: 4690630194
facebook-github-bot
pushed a commit
that referenced
this issue
Oct 22, 2019
Summary: Rules with failed runtime deps should fail even with --keep-going. try #1 here: D4222955 Reviewed By: rajyengi shipit-source-id: 7c9c41bfbb
facebook-github-bot
pushed a commit
that referenced
this issue
Jun 10, 2020
Summary: The objective of adding `IsolatedShellStep` is to migrate `ZipAlignStep` to an `IsolatedStep`. However, `ZipAlignStep` extends `ShellStep`, so it can't also extend `IsolatedStep`. **Options considered:** 1. Copy all the code from `ShellStep` into `ZipAlignStep` 2. Make `ShellStep` extend `IsolatedStep` 3. **[This diff]** Make an IsolatedShellStep that does what ShellStep currently does. `ShellStep` will hold an instance of `IsolatedShellStep` and delegate all calls to `IsolatedShellStep` (with the exceptions of `getShellCommand` and `getDescription`, which actually need `StepExecutionContext`) Chose option #3. Why not option #1: Requires maintaining the same code in two different places. Probably okay if the code is simple, but this code is non-trivial and error-prone to maintain separately. Why not option #2: This approach requires changing all the other subclasses of `ShellStep` to implement the new `IsolatedStep` methods. Choosing #3 avoids having to change a bunch of other subclasses **Implementation** *IsolatedShellStep* `IsolatedShellStep` is basically a copy of the old `ShellStep` with a few the following differences: * Instead of changing `execute()` directly to `executeIsolatedStep()`, I added a new `executeCommand()` method that both `ShellStep` and `IsolatedShellStep` can use. This is so code can be shared between the two after resolving their respective args into list of string * `executeCommand()` has a lot more arguments than `execute()` and `executeIsolatedSteps()` to propagate the overridden behavior from the subclasses properly. At some point this class should probably be refactored to have a bunch of `final` member vars that holds all these parameters. That's probably easier to maintain and understand. The current code flow is hard to follow. * Method visibilities changed to `public` so can be called by `ShellStep` * Added javadocs `IsolatedShellStep` is added underneath a new `common` dir in `buck/step/isolatedsteps`. I think `shell` rather than `common` could be a good name too. Any preferences? *ShellStep* * As aforementioned, `ShellStep` now holds an `IsolatedShellStep` and calls methods on `IsolatedShellStep` to avoid code duplication. * Note that the `IsolatedShellStep` it holds throws ann exception if `getShellCommandInternal(IsolatedExecutionContext context) ` is called. This is because if `ShellStep` is being used, `IsolatedShellStep`'s `getShellCommand()`, which in turn calls `getShellCommandInternal()`, should never be called due to context mismatch. --- With `IsolatedShellStep` in place, `ZipAlignStep` can be changed to extend `IsolatedShellStep` and be migrated to an `IsolatedStep` in a follow-up diff Reviewed By: mykola-semko shipit-source-id: 11a9f3727b840e8ef6890cbea3ad493279a28b36
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
While the first and third of those are laudable, your approach suffers from a fatal flaw: lack of readability. Documentation is useless if it cannot be understood.
Your examples:
echocommands to workI strongly encourage you to switch to more conventional documentation styles, so that your documentation is not an impediment to your objectives in releasing the project.
The text was updated successfully, but these errors were encountered: